You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2010/06/29 19:53:17 UTC

svn commit: r959051 - in /tomcat/tc5.5.x/trunk: ./ container/catalina/src/share/org/apache/catalina/valves/ container/webapps/docs/

Author: kkolinko
Date: Tue Jun 29 17:53:17 2010
New Revision: 959051

URL: http://svn.apache.org/viewvc?rev=959051&view=rev
Log:
Minor cleanups for AccessLogValve and FastCommonAccessLogValve classes
Reuse StringBuffer, use char instead of single-char String, etc.

Modified:
    tomcat/tc5.5.x/trunk/STATUS.txt
    tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java
    tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java
    tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml

Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=959051&r1=959050&r2=959051&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Tue Jun 29 17:53:17 2010
@@ -25,13 +25,6 @@ $Id$
 PATCHES PROPOSED TO BACKPORT:
   [ New proposals should be added at the end of the list ]
 
-* Minor cleanups for AccessLogValve classes
-  Reuses StringBuffer, uses char instead of single-char String, etc.
-  http://people.apache.org/~kkolinko/patches/2009-07-15_tc55_ALV.patch
-  http://people.apache.org/~kkolinko/patches/2009-07-15_tc55_FCALV.patch
-  +1: kkolinko, rjung, markt
-  -1:
-
 * Remove JSSE13Factory, JSSE13SocketFactory classes,
   because
     - TC 5.5 runs on JRE 1.4+ and that comes bundled with JSSE 1.4,

Modified: tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java?rev=959051&r1=959050&r2=959051&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java (original)
+++ tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/AccessLogValve.java Tue Jun 29 17:53:17 2010
@@ -119,7 +119,7 @@ public class AccessLogValve
     // ----------------------------------------------------------- Constructors
 
 
-    private static final String MARK_EMPTY = "-";
+    private static final char MARK_EMPTY = '-';
 
     /**
      * Construct a new instance of this class with default property values.
@@ -333,7 +333,7 @@ public class AccessLogValve
     /**
      * When formatting log lines, we often use strings like this one (" ").
      */
-    private String space = " ";
+    private static final char space = ' ';
 
 
     /**
@@ -591,11 +591,11 @@ public class AccessLogValve
 
         AccessDateStruct struct = (AccessDateStruct) currentDateStruct.get();
         Date date = struct.getDate();
-        StringBuffer result = new StringBuffer();
+        StringBuffer result = new StringBuffer(128);
 
         // Check to see if we should log using the "common" access log pattern
         if (common || combined) {
-            String value = null;
+            String value;
 
             if (isResolveHosts())
                 result.append(request.getRemoteHost());
@@ -632,29 +632,28 @@ public class AccessLogValve
 
             long length = response.getContentCountLong() ;
             if (length <= 0)
-                value = MARK_EMPTY;
+                result.append(MARK_EMPTY);
             else
-                value = "" + length;
-            result.append(value);
+                result.append(length);
 
             if (combined) {
                 result.append(space);
-                result.append("\"");
+                result.append('\"');
                 String referer = request.getHeader("referer");
                 if(referer != null)
                     result.append(referer);
                 else
                     result.append(MARK_EMPTY);
-                result.append("\"");
+                result.append('\"');
 
                 result.append(space);
-                result.append("\"");
+                result.append('\"');
                 String ua = request.getHeader("user-agent");
                 if(ua != null)
                     result.append(ua);
                 else
                     result.append(MARK_EMPTY);
-                result.append("\"");
+                result.append('\"');
             }
 
         } else {
@@ -667,26 +666,23 @@ public class AccessLogValve
                      * do not enounter a closing } - then I ignore the {
                      */
                     if ('{' == ch){
-                        StringBuffer name = new StringBuffer();
                         int j = i + 1;
                         for(;j < pattern.length() && '}' != pattern.charAt(j); j++) {
-                            name.append(pattern.charAt(j));
+                            // loop through characters that are part of the name
                         }
                         if (j+1 < pattern.length()) {
                             /* the +1 was to account for } which we increment now */
                             j++;
-                            result.append(replace(name.toString(),
-                                                pattern.charAt(j),
-                                                request,
-                                                response));
+                            replace(result, pattern.substring(i + 1, j - 1),
+                                    pattern.charAt(j), request, response);
                             i=j; /*Since we walked more than one character*/
                         } else {
                             //D'oh - end of string - pretend we never did this
                             //and do processing the "old way"
-                            result.append(replace(ch, struct, request, response, time));
+                            replace(result, ch, struct, request, response, time);
                         }
                     } else {
-                        result.append(replace(ch, struct, request, response,time ));
+                        replace(result, ch, struct, request, response,time );
                     }
                     replace = false;
                 } else if (ch == '%') {
@@ -812,18 +808,20 @@ public class AccessLogValve
 
 
     /**
-     * Return the replacement text for the specified pattern character.
+     * Print the replacement text for the specified pattern character.
      *
+     * @param result StringBuffer that accumulates the log message text
      * @param pattern Pattern character identifying the desired text
      * @param struct the object containing current Date so that this method
      *        doesn't need to create one
      * @param request Request being processed
      * @param response Response being processed
      */
-    private String replace(char pattern, AccessDateStruct struct, Request request,
-                           Response response, long time) {
+    private void replace(StringBuffer result, char pattern,
+            AccessDateStruct struct, Request request, Response response,
+            long time) {
 
-        String value = null;
+        String value;
 
         if (pattern == 'a') {
             value = request.getRemoteAddr();
@@ -836,134 +834,151 @@ public class AccessLogValve
         } else if (pattern == 'b') {
             long length = response.getContentCountLong() ;
             if (length <= 0)
-                value = MARK_EMPTY;
+                result.append(MARK_EMPTY);
             else
-                value = "" + length;
+                result.append(length);
+            return;
         } else if (pattern == 'B') {
-            value = "" + response.getContentCountLong();
+            result.append(response.getContentCountLong());
+            return;
         } else if (pattern == 'h') {
             value = request.getRemoteHost();
         } else if (pattern == 'H') {
             value = request.getProtocol();
         } else if (pattern == 'l') {
-            value = MARK_EMPTY;
+            result.append(MARK_EMPTY);
+            return;
         } else if (pattern == 'm') {
             if (request != null)
                 value = request.getMethod();
             else
                 value = "";
         } else if (pattern == 'p') {
-            value = "" + request.getServerPort();
+            result.append(request.getServerPort());
+            return;
         } else if (pattern == 'D') {
-                    value = "" + time;
+            result.append(time);
+            return;
         } else if (pattern == 'q') {
             String query = null;
             if (request != null)
                 query = request.getQueryString();
             if (query != null)
-                value = "?" + query;
-            else
-                value = "";
+                result.append('?').append(query);
+            return;
         } else if (pattern == 'r') {
-            StringBuffer sb = new StringBuffer();
             if (request != null) {
-                sb.append(request.getMethod());
-                sb.append(space);
-                sb.append(request.getRequestURI());
+                result.append(request.getMethod());
+                result.append(space);
+                result.append(request.getRequestURI());
                 if (request.getQueryString() != null) {
-                    sb.append('?');
-                    sb.append(request.getQueryString());
+                    result.append('?');
+                    result.append(request.getQueryString());
                 }
-                sb.append(space);
-                sb.append(request.getProtocol());
+                result.append(space);
+                result.append(request.getProtocol());
             } else {
-                sb.append("- - -");
+                result.append("- - -");
             }
-            value = sb.toString();
+            return;
         } else if (pattern == 'S') {
-            if (request != null)
-                if (request.getSession(false) != null)
-                    value = request.getSessionInternal(false).getIdInternal();
-                else value = MARK_EMPTY;
-            else
-                value = MARK_EMPTY;
+            if (request != null) {
+                if (request.getSession(false) != null) {
+                    result.append(request.getSessionInternal(false)
+                            .getIdInternal());
+                    return;
+                }
+            }
+            result.append(MARK_EMPTY);
+            return;
         } else if (pattern == 's') {
             if (response != null)
-                value = "" + response.getStatus();
+                result.append(response.getStatus());
             else
-                value = MARK_EMPTY;
+                result.append(MARK_EMPTY);
+            return;
         } else if (pattern == 't') {
-            value = struct.getCurrentDateString();
+            result.append(struct.getCurrentDateString());
+            return;
         } else if (pattern == 'T') {
-            value = struct.formatTimeTaken(time);
+            result.append(struct.formatTimeTaken(time));
+            return;
         } else if (pattern == 'u') {
-            if (request != null)
+            if (request != null) {
                 value = request.getRemoteUser();
-            if (value == null)
-                value = MARK_EMPTY;
+                if (value != null) {
+                    result.append(value);
+                    return;
+                }
+            }
+            result.append(MARK_EMPTY);
+            return;
         } else if (pattern == 'U') {
             if (request != null)
                 value = request.getRequestURI();
-            else
-                value = MARK_EMPTY;
+            else {
+                result.append(MARK_EMPTY);
+                return;
+            }
         } else if (pattern == 'v') {
             value = request.getServerName();
         } else if (pattern == 'I' ) {
             RequestInfo info = request.getCoyoteRequest().getRequestProcessor();
             if(info != null) {
-                value= info.getWorkerThreadName();
+                value = info.getWorkerThreadName();
             } else {
-                value = MARK_EMPTY;
+                result.append(MARK_EMPTY);
+                return;
             }
         } else {
-            value = "???" + pattern + "???";
+            result.append("???").append(pattern).append("???");
+            return;
         }
 
-        if (value == null)
-            return ("");
-        else
-            return (value);
-
+        if (value != null)
+            result.append(value);
     }
 
 
     /**
-     * Return the replacement text for the specified "header/parameter".
+     * Print the replacement text for the specified "header/parameter".
      *
+     * @param result StringBuffer that accumulates the log message text
      * @param header The header/parameter to get
      * @param type Where to get it from i=input,c=cookie,r=ServletRequest,s=Session
      * @param request Request being processed
      * @param response Response being processed
      */
-    private String replace(String header, char type, Request request,
-                           Response response) {
+    private void replace(StringBuffer result, String header, char type,
+            Request request, Response response) {
 
-        Object value = null;
+        Object value;
 
         switch (type) {
             case 'i':
                 if (null != request)
                     value = request.getHeader(header);
                 else
-                    value= "??";
+                    value = "??";
                 break;
             case 'o':
                 if (null != response) {
                     String[] values = response.getHeaderValues(header);
                     if(values.length > 0) {
-                        StringBuffer buf = new StringBuffer() ;
                         for (int i = 0; i < values.length; i++) {
                             String string = values[i];
-                            buf.append(string) ;
+                            result.append(string);
                             if(i+1<values.length)
-                                buf.append(",");
+                                result.append(',');
                         }
-                        value = buf.toString();
+                        return;
                     }
+                    value = null;
                 } else
-                    value= "??";
+                    value = "??";
                 break;
             case 'c':
+                value = null;
                  Cookie[] c = request.getCookies();
                  for (int i=0; c != null && i < c.length; i++){
                      if (header.equals(c[i].getName())){
@@ -979,6 +994,7 @@ public class AccessLogValve
                     value= "??";
                 break;
             case 's':
+                value = null;
                 if (null != request) {
                     HttpSession sess = request.getSession(false);
                     if (null != sess)
@@ -992,14 +1008,11 @@ public class AccessLogValve
         /* try catch in case toString() barfs */
         try {
             if (value!=null)
-                if (value instanceof String)
-                    return (String)value;
-                else
-                    return value.toString();
+                result.append(value.toString());
             else
-               return MARK_EMPTY;
+                result.append(MARK_EMPTY);
         } catch(Throwable e) {
-            return MARK_EMPTY;
+            result.append(MARK_EMPTY);
         }
     }
 
@@ -1016,21 +1029,21 @@ public class AccessLogValve
     private static String calculateTimeZoneOffset(long offset) {
         StringBuffer tz = new StringBuffer();
         if ((offset<0))  {
-            tz.append(MARK_EMPTY);
+            tz.append('-');
             offset = -offset;
         } else {
-            tz.append("+");
+            tz.append('+');
         }
 
         long hourOffset = offset/(1000*60*60);
         long minuteOffset = (offset/(1000*60)) % 60;
 
         if (hourOffset<10)
-            tz.append("0");
+            tz.append('0');
         tz.append(hourOffset);
 
         if (minuteOffset<10)
-            tz.append("0");
+            tz.append('0');
         tz.append(minuteOffset);
 
         return tz.toString();

Modified: tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java
URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java?rev=959051&r1=959050&r2=959051&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java (original)
+++ tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/valves/FastCommonAccessLogValve.java Tue Jun 29 17:53:17 2010
@@ -251,7 +251,7 @@ public final class FastCommonAccessLogVa
     /**
      * When formatting log lines, we often use strings like this one (" ").
      */
-    private String space = " ";
+    private static final char space = ' ';
 
 
     /**
@@ -504,7 +504,7 @@ public final class FastCommonAccessLogVa
             return;
         }
 
-        StringBuffer result = new StringBuffer();
+        StringBuffer result = new StringBuffer(128);
 
         // Check to see if we should log using the "common" access log pattern
         String value = null;
@@ -543,29 +543,28 @@ public final class FastCommonAccessLogVa
         
         long length = response.getContentCountLong() ;
         if (length <= 0)
-            value = "-";
+            result.append('-');
         else
-            value = "" + length;
-        result.append(value);
+            result.append(length);
         
         if (combined) {
             result.append(space);
-            result.append("\"");
+            result.append('\"');
             String referer = request.getHeader("referer");
             if(referer != null)
                 result.append(referer);
             else
-                result.append("-");
-            result.append("\"");
+                result.append('-');
+            result.append('\"');
             
             result.append(space);
-            result.append("\"");
+            result.append('\"');
             String ua = request.getHeader("user-agent");
             if(ua != null)
                 result.append(ua);
             else
-                result.append("-");
-            result.append("\"");
+                result.append('-');
+            result.append('\"');
         }
         
         log(result.toString());
@@ -695,21 +694,21 @@ public final class FastCommonAccessLogVa
     private static String calculateTimeZoneOffset(long offset) {
         StringBuffer tz = new StringBuffer();
         if ((offset<0))  {
-            tz.append("-");
+            tz.append('-');
             offset = -offset;
         } else {
-            tz.append("+");
+            tz.append('+');
         }
 
         long hourOffset = offset/(1000*60*60);
         long minuteOffset = (offset/(1000*60)) % 60;
 
         if (hourOffset<10)
-            tz.append("0");
+            tz.append('0');
         tz.append(hourOffset);
 
         if (minuteOffset<10)
-            tz.append("0");
+            tz.append('0');
         tz.append(minuteOffset);
 
         return tz.toString();

Modified: tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml?rev=959051&r1=959050&r2=959051&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml (original)
+++ tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml Tue Jun 29 17:53:17 2010
@@ -99,6 +99,10 @@
         <bug>49424</bug>: Avoid NPE if client provides no data with a chunked
         POST request. (markt)
       </fix>
+      <fix>
+        Minor code cleanup in AccessLogValve and FastCommonAccessLogValve
+        classes. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org