You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2022/03/28 22:10:21 UTC

[logging-log4j2] 06/07: Refactor common DOM parsing code.

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 69cf08345e72872487061acd7d00f6c569a71dc9
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Feb 5 08:30:33 2022 -0500

    Refactor common DOM parsing code.
    
    Conflicts:
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java
    	log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java
---
 .../org/apache/log4j/builders/AbstractBuilder.java | 45 ++++++++--
 .../builders/appender/AsyncAppenderBuilder.java    | 81 ++++++++----------
 .../builders/appender/ConsoleAppenderBuilder.java  | 57 +++++--------
 .../appender/DailyRollingFileAppenderBuilder.java  | 95 +++++++--------------
 .../builders/appender/FileAppenderBuilder.java     | 77 +++++------------
 .../builders/appender/NullAppenderBuilder.java     |  4 +-
 .../builders/appender/RewriteAppenderBuilder.java  | 48 +++++------
 .../appender/RollingFileAppenderBuilder.java       | 97 ++++++----------------
 8 files changed, 193 insertions(+), 311 deletions(-)

diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
index 50b3ec1..04b439c 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/AbstractBuilder.java
@@ -24,6 +24,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.log4j.bridge.FilterAdapter;
 import org.apache.log4j.bridge.FilterWrapper;
@@ -74,10 +77,8 @@ public abstract class AbstractBuilder implements Builder {
     protected org.apache.logging.log4j.core.Filter buildFilters(final String level, final Filter filter) {
         if (level != null && filter != null) {
             final List<org.apache.logging.log4j.core.Filter> filterList = new ArrayList<>();
-            final org.apache.logging.log4j.core.Filter thresholdFilter =
-                    ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE),
-                            org.apache.logging.log4j.core.Filter.Result.NEUTRAL,
-                            org.apache.logging.log4j.core.Filter.Result.DENY);
+            final org.apache.logging.log4j.core.Filter thresholdFilter = ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE),
+                org.apache.logging.log4j.core.Filter.Result.NEUTRAL, org.apache.logging.log4j.core.Filter.Result.DENY);
             filterList.add(thresholdFilter);
             Filter f = filter;
             while (f != null) {
@@ -90,9 +91,8 @@ public abstract class AbstractBuilder implements Builder {
             }
             return CompositeFilter.createFilters(filterList.toArray(org.apache.logging.log4j.core.Filter.EMPTY_ARRAY));
         } else if (level != null) {
-            return ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE),
-                    org.apache.logging.log4j.core.Filter.Result.NEUTRAL,
-                    org.apache.logging.log4j.core.Filter.Result.DENY);
+            return ThresholdFilter.createFilter(OptionConverter.convertLevel(level, Level.TRACE), org.apache.logging.log4j.core.Filter.Result.NEUTRAL,
+                org.apache.logging.log4j.core.Filter.Result.DENY);
         } else if (filter != null) {
             if (filter instanceof FilterWrapper) {
                 return ((FilterWrapper) filter).getFilter();
@@ -185,4 +185,35 @@ public abstract class AbstractBuilder implements Builder {
         chars[0] = Character.toLowerCase(chars[0]);
         return new String(chars);
     }
+
+    protected void setBoolean(final String name, final Element element, Holder<Boolean> ref) {
+        final String value = getValueAttribute(element);
+        if (value == null) {
+            LOGGER.warn("No value for {} parameter, using default {}", name, ref);
+        } else {
+            ref.set(Boolean.parseBoolean(value));
+        }
+    }
+
+    protected void setInteger(final String name, final Element element, Holder<Integer> ref) {
+        final String value = getValueAttribute(element);
+        if (value == null) {
+            LOGGER.warn("No value for {} parameter, using default {}", name, ref);
+        } else {
+            try {
+                ref.set(Integer.parseInt(value));
+            } catch (NumberFormatException e) {
+                LOGGER.warn("{} parsing {} parameter, using default {}: {}", e.getClass().getName(), name, ref, e.getMessage(), e);
+            }
+        }
+    }
+
+    protected void setString(final String name, final Element element, Holder<String> ref) {
+        final String value = getValueAttribute(element);
+        if (value == null) {
+            LOGGER.warn("No value for {} parameter, using default {}", name, ref);
+        } else {
+            ref.set(value);
+        }
+    }
 }
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java
index 8657156..4b8b17e 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/AsyncAppenderBuilder.java
@@ -16,6 +16,19 @@
  */
 package org.apache.log4j.builders.appender;
 
+import static org.apache.log4j.builders.BuilderManager.CATEGORY;
+import static org.apache.log4j.config.Log4j1Configuration.APPENDER_REF_TAG;
+import static org.apache.log4j.config.Log4j1Configuration.THRESHOLD_PARAM;
+import static org.apache.log4j.xml.XmlConfiguration.PARAM_TAG;
+import static org.apache.log4j.xml.XmlConfiguration.forEachElement;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.apache.log4j.Appender;
 import org.apache.log4j.bridge.AppenderWrapper;
 import org.apache.log4j.builders.AbstractBuilder;
@@ -58,7 +71,7 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui
     public AsyncAppenderBuilder() {
     }
 
-    public AsyncAppenderBuilder(String prefix, Properties props) {
+    public AsyncAppenderBuilder(final String prefix, final Properties props) {
         super(prefix, props);
     }
 
@@ -73,49 +86,25 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui
         forEachElement(appenderElement.getChildNodes(), (currentElement) -> {
             switch (currentElement.getTagName()) {
                 case APPENDER_REF_TAG:
-                    Appender appender = config.findAppenderByReference(currentElement);
+                    final Appender appender = config.findAppenderByReference(currentElement);
                     if (appender != null) {
                         appenderRefs.get().add(appender.getName());
                     }
                     break;
                 case PARAM_TAG: {
                     switch (getNameAttributeKey(currentElement)) {
-                        case BUFFER_SIZE_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for BufferSize parameter. Defaulting to 1024.");
-                            } else {
-                                bufferSize.set(Integer.parseInt(value));
-                            }
+                        case BUFFER_SIZE_PARAM:
+                            setInteger(BUFFER_SIZE_PARAM, currentElement, bufferSize);
                             break;
-                        }
-                        case BLOCKING_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Blocking parameter. Defaulting to false.");
-                            } else {
-                                blocking.set(Boolean.parseBoolean(value));
-                            }
+                        case BLOCKING_PARAM:
+                            setBoolean(BLOCKING_PARAM, currentElement, blocking);
                             break;
-                        }
-                        case INCLUDE_LOCATION_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for IncludeLocation parameter. Defaulting to false.");
-                            } else {
-                                includeLocation.set(Boolean.parseBoolean(value));
-                            }
+                        case INCLUDE_LOCATION_PARAM:
+                            setBoolean(INCLUDE_LOCATION_PARAM, currentElement, includeLocation);
                             break;
-                        }
-                        case THRESHOLD_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                            } else {
-                                level.set(value);
-                            }
+                        case THRESHOLD_PARAM:
+                            setString(THRESHOLD_PARAM, currentElement, level);
                             break;
-                        }
                     }
                     break;
                 }
@@ -128,16 +117,16 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        String appenderRef = getProperty(APPENDER_REF_TAG);
-        boolean blocking = getBooleanProperty(BLOCKING_PARAM);
-        boolean includeLocation = getBooleanProperty(INCLUDE_LOCATION_PARAM);
-        String level = getProperty(THRESHOLD_PARAM);
-        int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 1024);
+        final String appenderRef = getProperty(APPENDER_REF_TAG);
+        final boolean blocking = getBooleanProperty(BLOCKING_PARAM);
+        final boolean includeLocation = getBooleanProperty(INCLUDE_LOCATION_PARAM);
+        final String level = getProperty(THRESHOLD_PARAM);
+        final int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 1024);
         if (appenderRef == null) {
             LOGGER.warn("No appender references configured for AsyncAppender {}", name);
             return null;
         }
-        Appender appender = configuration.parseAppender(props, appenderRef);
+        final Appender appender = configuration.parseAppender(props, appenderRef);
         if (appender == null) {
             LOGGER.warn("Cannot locate Appender {}", appenderRef);
             return null;
@@ -146,14 +135,14 @@ public class AsyncAppenderBuilder extends AbstractBuilder implements AppenderBui
                 configuration);
     }
 
-    private <T extends Log4j1Configuration> Appender createAppender(String name, String level,
-            String[] appenderRefs, boolean blocking, int bufferSize, boolean includeLocation,
-            T configuration) {
-        org.apache.logging.log4j.Level logLevel = OptionConverter.convertLevel(level,
+    private <T extends Log4j1Configuration> Appender createAppender(final String name, final String level,
+            final String[] appenderRefs, final boolean blocking, final int bufferSize, final boolean includeLocation,
+            final T configuration) {
+        final org.apache.logging.log4j.Level logLevel = OptionConverter.convertLevel(level,
                 org.apache.logging.log4j.Level.TRACE);
-        AppenderRef[] refs = new AppenderRef[appenderRefs.length];
+        final AppenderRef[] refs = new AppenderRef[appenderRefs.length];
         int index = 0;
-        for (String appenderRef : appenderRefs) {
+        for (final String appenderRef : appenderRefs) {
             refs[index++] = AppenderRef.createAppenderRef(appenderRef, logLevel, null);
         }
         return new AppenderWrapper(AsyncAppender.newBuilder()
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
index 78c1420..aa8056e 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/ConsoleAppenderBuilder.java
@@ -85,8 +85,8 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB
                     break;
                 case PARAM_TAG: {
                     switch (getNameAttributeKey(currentElement)) {
-                        case TARGET_PARAM: {
-                            String value = getValueAttribute(currentElement);
+                        case TARGET_PARAM:
+                            final String value = getValueAttribute(currentElement);
                             if (value == null) {
                                 LOGGER.warn("No value supplied for target parameter. Defaulting to " + SYSTEM_OUT);
                             } else {
@@ -102,34 +102,15 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB
                                 }
                             }
                             break;
-                        }
-                        case THRESHOLD_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                            } else {
-                                level.set(value);
-                            }
+                        case THRESHOLD_PARAM:
+                            setString(THRESHOLD_PARAM, currentElement, level);
                             break;
-                        }
-                        case FOLLOW_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Follow parameter. Using default of {}", follow);
-                            } else {
-                                follow.set(Boolean.parseBoolean(value));
-                            }
+                        case FOLLOW_PARAM:
+                            setBoolean(FOLLOW_PARAM, currentElement, follow);
                             break;
-                        }
-                        case IMMEDIATE_FLUSH_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", immediateFlush);
-                            } else {
-                                immediateFlush.set(Boolean.parseBoolean(value));
-                            }
+                        case IMMEDIATE_FLUSH_PARAM:
+                            setBoolean(IMMEDIATE_FLUSH_PARAM, currentElement, immediateFlush);
                             break;
-                        }
                     }
                     break;
                 }
@@ -137,7 +118,7 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB
         });
         Filter head = null;
         Filter current = null;
-        for (Filter f : filters.get()) {
+        for (final Filter f : filters.get()) {
             if (head == null) {
                 head = f;
                 current = f;
@@ -152,17 +133,17 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        Layout layout = configuration.parseLayout(layoutPrefix, name, props);
-        Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
-        String level = getProperty(THRESHOLD_PARAM);
-        String target = getProperty(TARGET_PARAM);
-        boolean follow = getBooleanProperty(FOLLOW_PARAM);
-        boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM);
+        final Layout layout = configuration.parseLayout(layoutPrefix, name, props);
+        final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
+        final String level = getProperty(THRESHOLD_PARAM);
+        final String target = getProperty(TARGET_PARAM);
+        final boolean follow = getBooleanProperty(FOLLOW_PARAM);
+        final boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM);
         return createAppender(name, layout, filter, level, target, immediateFlush, follow, configuration);
     }
 
-    private <T extends Log4j1Configuration> Appender createAppender(String name, Layout layout, Filter filter,
-            String level, String target, boolean immediateFlush, boolean follow, T configuration) {
+    private <T extends Log4j1Configuration> Appender createAppender(final String name, final Layout layout, final Filter filter,
+            final String level, final String target, final boolean immediateFlush, final boolean follow, final T configuration) {
         org.apache.logging.log4j.core.Layout<?> consoleLayout = null;
 
         if (layout instanceof LayoutWrapper) {
@@ -170,8 +151,8 @@ public class ConsoleAppenderBuilder extends AbstractBuilder implements AppenderB
         } else if (layout != null) {
             consoleLayout = new LayoutAdapter(layout);
         }
-        org.apache.logging.log4j.core.Filter consoleFilter = buildFilters(level, filter);
-        ConsoleAppender.Target consoleTarget = SYSTEM_ERR.equals(target)
+        final org.apache.logging.log4j.core.Filter consoleFilter = buildFilters(level, filter);
+        final ConsoleAppender.Target consoleTarget = SYSTEM_ERR.equals(target)
                 ? ConsoleAppender.Target.SYSTEM_ERR : ConsoleAppender.Target.SYSTEM_OUT;
         return new AppenderWrapper(ConsoleAppender.newBuilder()
                 .setName(name)
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java
index 8ee058b..2ccc38c 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/DailyRollingFileAppenderBuilder.java
@@ -60,7 +60,7 @@ public class DailyRollingFileAppenderBuilder extends AbstractBuilder implements
     public DailyRollingFileAppenderBuilder() {
     }
 
-    public DailyRollingFileAppenderBuilder(String prefix, Properties props) {
+    public DailyRollingFileAppenderBuilder(final String prefix, final Properties props) {
         super(prefix, props);
     }
 
@@ -84,68 +84,31 @@ public class DailyRollingFileAppenderBuilder extends AbstractBuilder implements
                 case FILTER_TAG:
                     filter.set(config.parseFilters(currentElement));
                     break;
-                case PARAM_TAG: {
+                case PARAM_TAG:
                     switch (getNameAttributeKey(currentElement)) {
                         case FILE_PARAM:
-                            fileName.set(getValueAttribute(currentElement));
+                            setString(FILE_PARAM, currentElement, fileName);
                             break;
-                        case APPEND_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                append.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for append parameter");
-                            }
+                        case APPEND_PARAM:
+                            setBoolean(APPEND_PARAM, currentElement, append);
                             break;
-                        }
-                        case BUFFERED_IO_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                bufferedIo.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for bufferedIo parameter");
-                            }
+                        case BUFFERED_IO_PARAM:
+                            setBoolean(BUFFERED_IO_PARAM, currentElement, bufferedIo);
                             break;
-                        }
-                        case BUFFER_SIZE_PARAM: {
-                            String size = getValueAttribute(currentElement);
-                            if (size != null) {
-                                bufferSize.set(Integer.parseInt(size));
-                            } else {
-                                LOGGER.warn("No value provide for bufferSize parameter");
-                            }
+                        case BUFFER_SIZE_PARAM:
+                            setInteger(BUFFER_SIZE_PARAM, currentElement, bufferSize);
                             break;
-                        }
-                        case THRESHOLD_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                            } else {
-                                level.set(value);
-                            }
+                        case THRESHOLD_PARAM:
+                            setString(THRESHOLD_PARAM, currentElement, level);
                             break;
-                        }
-                        case DATE_PATTERN_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for DatePattern parameter, ignoring.");
-                            } else {
-                                datePattern.set(value);
-                            }
+                        case DATE_PATTERN_PARAM:
+                            setString(DATE_PATTERN_PARAM, currentElement, datePattern);
                             break;
-                        }
-                        case IMMEDIATE_FLUSH_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", true);
-                            } else {
-                                immediateFlush.set(Boolean.getBoolean(value));
-                            }
+                        case IMMEDIATE_FLUSH_PARAM:
+                            setBoolean(IMMEDIATE_FLUSH_PARAM, currentElement, immediateFlush);
                             break;
-                        }
                     }
                     break;
-                }
             }
         });
         return createAppender(name, layout.get(), filter.get(), fileName.get(), append.get(), immediateFlush.get(),
@@ -156,15 +119,15 @@ public class DailyRollingFileAppenderBuilder extends AbstractBuilder implements
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        Layout layout = configuration.parseLayout(layoutPrefix, name, props);
-        Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
-        String fileName = getProperty(FILE_PARAM);
-        String level = getProperty(THRESHOLD_PARAM);
-        boolean append = getBooleanProperty(APPEND_PARAM, true);
-        boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
-        boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM, false);
-        int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 8192);
-        String datePattern = getProperty(DATE_PATTERN_PARAM, DEFAULT_DATE_PATTERN);
+        final Layout layout = configuration.parseLayout(layoutPrefix, name, props);
+        final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
+        final String fileName = getProperty(FILE_PARAM);
+        final String level = getProperty(THRESHOLD_PARAM);
+        final boolean append = getBooleanProperty(APPEND_PARAM, true);
+        final boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
+        final boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM, false);
+        final int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 8192);
+        final String datePattern = getProperty(DATE_PATTERN_PARAM, DEFAULT_DATE_PATTERN);
         return createAppender(name, layout, filter, fileName, append, immediateFlush, level, bufferedIo, bufferSize,
                 datePattern, configuration, configuration.getComponent(Clock.KEY));
     }
@@ -183,15 +146,15 @@ public class DailyRollingFileAppenderBuilder extends AbstractBuilder implements
         } else if (layout != null) {
             fileLayout = new LayoutAdapter(layout);
         }
-        org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
+        final org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
         if (fileName == null) {
             LOGGER.warn("Unable to create File Appender, no file name provided");
             return null;
         }
-        String filePattern = fileName + "%d{" + datePattern + "}";
-        TriggeringPolicy timePolicy = TimeBasedTriggeringPolicy.newBuilder().setClock(clock).setModulate(true).build();
-        TriggeringPolicy policy = CompositeTriggeringPolicy.createPolicy(timePolicy);
-        RolloverStrategy strategy = DefaultRolloverStrategy.newBuilder()
+        final String filePattern = fileName + "%d{" + datePattern + "}";
+        final TriggeringPolicy timePolicy = TimeBasedTriggeringPolicy.newBuilder().setClock(clock).setModulate(true).build();
+        final TriggeringPolicy policy = CompositeTriggeringPolicy.createPolicy(timePolicy);
+        final RolloverStrategy strategy = DefaultRolloverStrategy.newBuilder()
                 .setConfig(configuration)
                 .setMax(Integer.toString(Integer.MAX_VALUE))
                 .build();
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
index 2e3e9a4..96d2f90 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/FileAppenderBuilder.java
@@ -56,7 +56,7 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil
     public FileAppenderBuilder() {
     }
 
-    public FileAppenderBuilder(String prefix, Properties props) {
+    public FileAppenderBuilder(final String prefix, final Properties props) {
         super(prefix, props);
     }
 
@@ -79,59 +79,28 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil
                 case FILTER_TAG:
                     filter.set(config.parseFilters(currentElement));
                     break;
-                case PARAM_TAG: {
+                case PARAM_TAG:
                     switch (getNameAttributeKey(currentElement)) {
                         case FILE_PARAM:
-                            fileName.set(getValueAttribute(currentElement));
+                            setString(FILE_PARAM, currentElement, fileName);
                             break;
-                        case APPEND_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                append.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for append parameter");
-                            }
+                        case APPEND_PARAM:
+                            setBoolean(APPEND_PARAM, currentElement, append);
                             break;
-                        }
-                        case BUFFERED_IO_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                bufferedIo.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for bufferedIo parameter");
-                            }
+                        case BUFFERED_IO_PARAM:
+                            setBoolean(BUFFERED_IO_PARAM, currentElement, bufferedIo);
                             break;
-                        }
-                        case BUFFER_SIZE_PARAM: {
-                            String size = getValueAttribute(currentElement);
-                            if (size != null) {
-                                bufferSize.set(Integer.parseInt(size));
-                            } else {
-                                LOGGER.warn("No value provide for bufferSize parameter");
-                            }
+                        case BUFFER_SIZE_PARAM:
+                            setInteger(BUFFER_SIZE_PARAM, currentElement, bufferSize);
                             break;
-                        }
-                        case THRESHOLD_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                            } else {
-                                level.set(value);
-                            }
+                        case THRESHOLD_PARAM:
+                            setString(THRESHOLD_PARAM, currentElement, level);
                             break;
-                        }
-                        case IMMEDIATE_FLUSH_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", true);
-                            } else {
-                                immediateFlush.set(Boolean.getBoolean(value));
-                            }
+                        case IMMEDIATE_FLUSH_PARAM:
+                            setBoolean(IMMEDIATE_FLUSH_PARAM, currentElement, immediateFlush);
                             break;
-                        }
                     }
                     break;
-                }
             }
         });
 
@@ -142,20 +111,20 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        Layout layout = configuration.parseLayout(layoutPrefix, name, props);
-        Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
-        String level = getProperty(THRESHOLD_PARAM);
-        String fileName = getProperty(FILE_PARAM);
-        boolean append = getBooleanProperty(APPEND_PARAM, true);
-        boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
-        boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM, false);
-        int bufferSize = Integer.parseInt(getProperty(BUFFER_SIZE_PARAM, "8192"));
+        final Layout layout = configuration.parseLayout(layoutPrefix, name, props);
+        final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
+        final String level = getProperty(THRESHOLD_PARAM);
+        final String fileName = getProperty(FILE_PARAM);
+        final boolean append = getBooleanProperty(APPEND_PARAM, true);
+        final boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
+        final boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM, false);
+        final int bufferSize = Integer.parseInt(getProperty(BUFFER_SIZE_PARAM, "8192"));
         return createAppender(name, configuration, layout, filter, fileName, level, immediateFlush,
                 append, bufferedIo, bufferSize);
     }
 
     private Appender createAppender(final String name, final Log4j1Configuration configuration, final Layout layout,
-            final Filter filter, final String fileName, String level, boolean immediateFlush, final boolean append,
+            final Filter filter, final String fileName, final String level, boolean immediateFlush, final boolean append,
             final boolean bufferedIo, final int bufferSize) {
         org.apache.logging.log4j.core.Layout<?> fileLayout = null;
         if (bufferedIo) {
@@ -166,7 +135,7 @@ public class FileAppenderBuilder extends AbstractBuilder implements AppenderBuil
         } else if (layout != null) {
             fileLayout = new LayoutAdapter(layout);
         }
-        org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
+        final org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
         if (fileName == null) {
             LOGGER.warn("Unable to create File Appender, no file name provided");
             return null;
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/NullAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/NullAppenderBuilder.java
index 07f2fd5..98af6a8 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/NullAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/NullAppenderBuilder.java
@@ -39,8 +39,8 @@ public class NullAppenderBuilder implements AppenderBuilder {
     private static final Logger LOGGER = StatusLogger.getLogger();
 
     @Override
-    public Appender parseAppender(Element appenderElement, XmlConfiguration config) {
-        String name = appenderElement.getAttribute("name");
+    public Appender parseAppender(final Element appenderElement, final XmlConfiguration config) {
+        final String name = appenderElement.getAttribute("name");
         return new AppenderWrapper(NullAppender.createAppender(name));
     }
 
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java
index 3cfff48..cac02ee 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RewriteAppenderBuilder.java
@@ -61,7 +61,7 @@ public class RewriteAppenderBuilder extends AbstractBuilder implements AppenderB
     public RewriteAppenderBuilder() {
     }
 
-    public RewriteAppenderBuilder(String prefix, Properties props) {
+    public RewriteAppenderBuilder(final String prefix, final Properties props) {
         super(prefix, props);
     }
 
@@ -75,33 +75,25 @@ public class RewriteAppenderBuilder extends AbstractBuilder implements AppenderB
         forEachElement(appenderElement.getChildNodes(), (currentElement) -> {
             switch (currentElement.getTagName()) {
                 case APPENDER_REF_TAG:
-                    Appender appender = config.findAppenderByReference(currentElement);
+                    final Appender appender = config.findAppenderByReference(currentElement);
                     if (appender != null) {
                         appenderRefs.get().add(appender.getName());
                     }
                     break;
-                case REWRITE_POLICY_TAG: {
-                    RewritePolicy policy = config.parseRewritePolicy(currentElement);
+                case REWRITE_POLICY_TAG:
+                    final RewritePolicy policy = config.parseRewritePolicy(currentElement);
                     if (policy != null) {
                         rewritePolicyHolder.set(policy);
                     }
                     break;
-                }
-                case FILTER_TAG: {
+                case FILTER_TAG:
                     filter.set(config.parseFilters(currentElement));
                     break;
-                }
-                case PARAM_TAG: {
+                case PARAM_TAG:
                     if (getNameAttributeKey(currentElement).equalsIgnoreCase(THRESHOLD_PARAM)) {
-                        String value = getValueAttribute(currentElement);
-                        if (value == null) {
-                            LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                        } else {
-                            level.set(value);
-                        }
+                        setString(THRESHOLD_PARAM, currentElement, level);
                     }
                     break;
-                }
             }
         });
         return createAppender(name, level.get(), appenderRefs.get().toArray(new String[0]), rewritePolicyHolder.get(),
@@ -111,18 +103,18 @@ public class RewriteAppenderBuilder extends AbstractBuilder implements AppenderB
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        String appenderRef = getProperty(APPENDER_REF_TAG);
-        Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
-        String policyPrefix = appenderPrefix + ".rewritePolicy";
-        String className = getProperty(policyPrefix);
-        RewritePolicy policy = configuration.getBuilderManager().parseRewritePolicy(className, policyPrefix,
+        final String appenderRef = getProperty(APPENDER_REF_TAG);
+        final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
+        final String policyPrefix = appenderPrefix + ".rewritePolicy";
+        final String className = getProperty(policyPrefix);
+        final RewritePolicy policy = configuration.getBuilderManager().parseRewritePolicy(className, policyPrefix,
                 props, configuration);
-        String level = getProperty(THRESHOLD_PARAM);
+        final String level = getProperty(THRESHOLD_PARAM);
         if (appenderRef == null) {
             LOGGER.warn("No appender references configured for AsyncAppender {}", name);
             return null;
         }
-        Appender appender = configuration.parseAppender(props, appenderRef);
+        final Appender appender = configuration.parseAppender(props, appenderRef);
         if (appender == null) {
             LOGGER.warn("Cannot locate Appender {}", appenderRef);
             return null;
@@ -130,16 +122,16 @@ public class RewriteAppenderBuilder extends AbstractBuilder implements AppenderB
         return createAppender(name, level, new String[] {appenderRef}, policy, filter, configuration);
     }
 
-    private <T extends Log4j1Configuration> Appender createAppender(String name, String level,
-            String[] appenderRefs, RewritePolicy policy, Filter filter, T configuration) {
-        org.apache.logging.log4j.Level logLevel = OptionConverter.convertLevel(level,
+    private <T extends Log4j1Configuration> Appender createAppender(final String name, final String level,
+            final String[] appenderRefs, final RewritePolicy policy, final Filter filter, final T configuration) {
+        final org.apache.logging.log4j.Level logLevel = OptionConverter.convertLevel(level,
                 org.apache.logging.log4j.Level.TRACE);
-        AppenderRef[] refs = new AppenderRef[appenderRefs.length];
+        final AppenderRef[] refs = new AppenderRef[appenderRefs.length];
         int index = 0;
-        for (String appenderRef : appenderRefs) {
+        for (final String appenderRef : appenderRefs) {
             refs[index++] = AppenderRef.createAppenderRef(appenderRef, logLevel, null);
         }
-        org.apache.logging.log4j.core.Filter rewriteFilter = buildFilters(level, filter);
+        final org.apache.logging.log4j.core.Filter rewriteFilter = buildFilters(level, filter);
         org.apache.logging.log4j.core.appender.rewrite.RewritePolicy rewritePolicy;
         if (policy instanceof RewritePolicyWrapper) {
             rewritePolicy = ((RewritePolicyWrapper) policy).getPolicy();
diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java
index 8923794..3a74245 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/builders/appender/RollingFileAppenderBuilder.java
@@ -86,77 +86,34 @@ public class RollingFileAppenderBuilder extends AbstractBuilder implements Appen
                 case FILTER_TAG:
                     filter.set(config.parseFilters(currentElement));
                     break;
-                case PARAM_TAG: {
+                case PARAM_TAG:
                     switch (getNameAttributeKey(currentElement)) {
                         case FILE_PARAM:
-                            fileName.set(getValueAttribute(currentElement));
+                            setString(FILE_PARAM, currentElement, fileName);
                             break;
-                        case APPEND_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                append.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for append parameter");
-                            }
+                        case APPEND_PARAM:
+                            setBoolean(APPEND_PARAM, currentElement, append);
                             break;
-                        }
-                        case BUFFERED_IO_PARAM: {
-                            String bool = getValueAttribute(currentElement);
-                            if (bool != null) {
-                                bufferedIo.set(Boolean.parseBoolean(bool));
-                            } else {
-                                LOGGER.warn("No value provided for bufferedIo parameter");
-                            }
+                        case BUFFERED_IO_PARAM:
+                            setBoolean(BUFFERED_IO_PARAM, currentElement, bufferedIo);
                             break;
-                        }
-                        case BUFFER_SIZE_PARAM: {
-                            String size = getValueAttribute(currentElement);
-                            if (size != null) {
-                                bufferSize.set(Integer.parseInt(size));
-                            } else {
-                                LOGGER.warn("No value provide for bufferSize parameter");
-                            }
+                        case BUFFER_SIZE_PARAM:
+                            setInteger(BUFFER_SIZE_PARAM, currentElement, bufferSize);
                             break;
-                        }
-                        case MAX_BACKUP_INDEX: {
-                            String size = getValueAttribute(currentElement);
-                            if (size != null) {
-                                maxBackups.set(size);
-                            } else {
-                                LOGGER.warn("No value provide for maxBackupIndex parameter");
-                            }
+                        case MAX_BACKUP_INDEX:
+                            setString(MAX_BACKUP_INDEX, currentElement, maxBackups);
                             break;
-                        }
-                        case MAX_SIZE_PARAM: {
-                            String size = getValueAttribute(currentElement);
-                            if (size != null) {
-                                maxSize.set(size);
-                            } else {
-                                LOGGER.warn("No value provide for bufferSize parameter");
-                            }
+                        case MAX_SIZE_PARAM:
+                            setString(MAX_SIZE_PARAM, currentElement, maxSize);
                             break;
-                        }
-                        case THRESHOLD_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for Threshold parameter, ignoring.");
-                            } else {
-                                level.set(value);
-                            }
+                        case THRESHOLD_PARAM:
+                            setString(THRESHOLD_PARAM, currentElement, level);
                             break;
-                        }
-                        case IMMEDIATE_FLUSH_PARAM: {
-                            String value = getValueAttribute(currentElement);
-                            if (value == null) {
-                                LOGGER.warn("No value supplied for ImmediateFlush parameter. Using default of {}", true);
-                            } else {
-                                immediateFlush.set(Boolean.getBoolean(value));
-                            }
+                        case IMMEDIATE_FLUSH_PARAM:
+                            setBoolean(IMMEDIATE_FLUSH_PARAM, currentElement, immediateFlush);
                             break;
-                        }
                     }
                     break;
-                }
             }
         });
         return createAppender(name, config, layout.get(), filter.get(), append.get(), bufferedIo.get(),
@@ -167,16 +124,16 @@ public class RollingFileAppenderBuilder extends AbstractBuilder implements Appen
     @Override
     public Appender parseAppender(final String name, final String appenderPrefix, final String layoutPrefix,
             final String filterPrefix, final Properties props, final PropertiesConfiguration configuration) {
-        Layout layout = configuration.parseLayout(layoutPrefix, name, props);
-        Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
-        String fileName = getProperty(FILE_PARAM);
-        String level = getProperty(THRESHOLD_PARAM);
-        boolean append = getBooleanProperty(APPEND_PARAM, true);
-        boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
-        boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM);
-        int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 8192);
-        String maxSize = getProperty(MAX_SIZE_PARAM, DEFAULT_MAX_SIZE);
-        String maxBackups = getProperty(MAX_BACKUP_INDEX, DEFAULT_MAX_BACKUPS);
+        final Layout layout = configuration.parseLayout(layoutPrefix, name, props);
+        final Filter filter = configuration.parseAppenderFilters(props, filterPrefix, name);
+        final String fileName = getProperty(FILE_PARAM);
+        final String level = getProperty(THRESHOLD_PARAM);
+        final boolean append = getBooleanProperty(APPEND_PARAM, true);
+        final boolean immediateFlush = getBooleanProperty(IMMEDIATE_FLUSH_PARAM, true);
+        final boolean bufferedIo = getBooleanProperty(BUFFERED_IO_PARAM, false);
+        final int bufferSize = getIntegerProperty(BUFFER_SIZE_PARAM, 8192);
+        final String maxSize = getProperty(MAX_SIZE_PARAM, DEFAULT_MAX_SIZE);
+        final String maxBackups = getProperty(MAX_BACKUP_INDEX, DEFAULT_MAX_BACKUPS);
         return createAppender(name, configuration, layout, filter, append, bufferedIo, bufferSize, immediateFlush,
                 fileName, level, maxSize, maxBackups, configuration.getComponent(Clock.KEY));
     }
@@ -194,7 +151,7 @@ public class RollingFileAppenderBuilder extends AbstractBuilder implements Appen
         } else if (layout != null) {
             fileLayout = new LayoutAdapter(layout);
         }
-        org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
+        final org.apache.logging.log4j.core.Filter fileFilter = buildFilters(level, filter);
         if (fileName == null) {
             LOGGER.warn("Unable to create File Appender, no file name provided");
             return null;