Clean up PropertyEnvironment

We clean up `PropertyEnvironment` from all the unused public methods. Since all the classes should only use `getProperty(Class)`, the other converter methods are not necessary. We keep `getStringProperty` (renamed to `getProperty(String)`) for convenience: this allows to transform a `PropertyEnvironment` into a `PropertySource`.
apache · May 22, 2024 · cd83a8c · cd83a8c
1 parent 141c584
commit cd83a8c
Show file tree

Hide file tree

Showing 18 changed files with 56 additions and 609 deletions.
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/OptionConverter.java
@@ -16,8 +16,6 @@
  */
 package org.apache.logging.log4j.core.util;
 
-import static org.apache.logging.log4j.util.Strings.toRootUpperCase;
-
 import java.io.InterruptedIOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -40,23 +38,12 @@ public final class OptionConverter {
     private static final char DELIM_STOP = '}';
     private static final int DELIM_START_LEN = 2;
     private static final int DELIM_STOP_LEN = 1;
-    private static final int ONE_K = 1024;
 
     /**
      * OptionConverter is a static class.
      */
     private OptionConverter() {}
 
-    public static String[] concatenateArrays(final String[] l, final String[] r) {
-        final int len = l.length + r.length;
-        final String[] a = new String[len];
-
-        System.arraycopy(l, 0, a, 0, l.length);
-        System.arraycopy(r, 0, a, l.length, r.length);
-
-        return a;
-    }
-
     public static String convertSpecialChars(final String s) {
         char c;
         final int len = s.length();
@@ -114,49 +101,6 @@ public static Object instantiateByKey(
         return OptionConverter.instantiateByClassName(className.trim(), superClass, defaultValue);
     }
 
-    /**
-     * If <code>value</code> is "true", then {@code true} is
-     * returned. If <code>value</code> is "false", then
-     * {@code false} is returned. Otherwise, <code>default</code> is
-     * returned.
-     *
-     * <p>Case of value is unimportant.</p>
-     * @param value The value to convert.
-     * @param defaultValue The default value.
-     * @return true or false, depending on the value and/or default.
-     */
-    public static boolean toBoolean(final String value, final boolean defaultValue) {
-        if (value == null) {
-            return defaultValue;
-        }
-        final String trimmedVal = value.trim();
-        if ("true".equalsIgnoreCase(trimmedVal)) {
-            return true;
-        }
-        if ("false".equalsIgnoreCase(trimmedVal)) {
-            return false;
-        }
-        return defaultValue;
-    }
-
-    /**
-     * Convert the String value to an int.
-     * @param value The value as a String.
-     * @param defaultValue The default value.
-     * @return The value as an int.
-     */
-    public static int toInt(final String value, final int defaultValue) {
-        if (value != null) {
-            final String s = value.trim();
-            try {
-                return Integer.parseInt(s);
-            } catch (final NumberFormatException e) {
-                LOGGER.error("[{}] is not in proper int form.", s, e);
-            }
-        }
-        return defaultValue;
-    }
-
     public static Level toLevel(String value, Level defaultValue) {
         if (value == null) {
             return defaultValue;
@@ -220,40 +164,6 @@ public static Level toLevel(String value, Level defaultValue) {
         return result;
     }
 
-    /**
-     *
-     * @param value The size of the file as a String.
-     * @param defaultValue The default value.
-     * @return The size of the file as a long.
-     */
-    public static long toFileSize(final String value, final long defaultValue) {
-        if (value == null) {
-            return defaultValue;
-        }
-
-        String str = toRootUpperCase(value.trim());
-        long multiplier = 1;
-        int index;
-
-        if ((index = str.indexOf("KB")) != -1) {
-            multiplier = ONE_K;
-            str = str.substring(0, index);
-        } else if ((index = str.indexOf("MB")) != -1) {
-            multiplier = ONE_K * ONE_K;
-            str = str.substring(0, index);
-        } else if ((index = str.indexOf("GB")) != -1) {
-            multiplier = ONE_K * ONE_K * ONE_K;
-            str = str.substring(0, index);
-        }
-        try {
-            return Long.parseLong(str) * multiplier;
-        } catch (final NumberFormatException e) {
-            LOGGER.error("[{}] is not in proper int form.", str);
-            LOGGER.error("[{}] not in expected format.", value, e);
-        }
-        return defaultValue;
-    }
-
     /**
      * Find the value corresponding to <code>key</code> in
      * <code>props</code>. Then perform variable substitution on the
@@ -380,7 +290,7 @@ private static String substVars(final String val, final Properties props, final
             j += DELIM_START_LEN;
             final String key = val.substring(j, k);
             // first try in System properties
-            String replacement = PropertyEnvironment.getGlobal().getStringProperty(key, null);
+            String replacement = PropertyEnvironment.getGlobal().getProperty(key);
             // then try props parameter
             if (replacement == null && props != null) {
                 replacement = props.getProperty(key);

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
@@ -21,7 +21,7 @@
 import java.util.Random;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.logging.log4j.core.impl.CoreProperties;
+import org.apache.logging.log4j.core.impl.CoreProperties.UuidProperties;
 import org.apache.logging.log4j.kit.env.PropertyEnvironment;
 
 /**
@@ -37,9 +37,8 @@ public final class UuidUtil {
     private static final byte VARIANT = (byte) 0x80;
     private static final int SEQUENCE_MASK = 0x3FFF;
     private static final long NUM_100NS_INTERVALS_SINCE_UUID_EPOCH = 0x01b21dd213814000L;
-    private static final long INITIAL_UUID_SEQNO = PropertyEnvironment.getGlobal()
-            .getProperty(CoreProperties.UuidProperties.class)
-            .sequence();
+    private static final long INITIAL_UUID_SEQNO =
+            PropertyEnvironment.getGlobal().getProperty(UuidProperties.class).sequence();
 
     private static final long LOW_MASK = 0xffffffffL;
     private static final long MID_MASK = 0xffff00000000L;
@@ -78,7 +77,7 @@ static long initialize(byte[] mac) {
         System.arraycopy(mac, index, node, 2, length);
         final ByteBuffer buf = ByteBuffer.wrap(node);
         long rand = INITIAL_UUID_SEQNO;
-        String assigned = PropertyEnvironment.getGlobal().getStringProperty(ASSIGNED_SEQUENCES);
+        String assigned = System.getProperty(ASSIGNED_SEQUENCES);
         final long[] sequences;
         if (assigned == null) {
             sequences = new long[0];

diff --git a/log4j-docker/src/main/java/org/apache/logging/log4j/docker/DockerLookup.java b/log4j-docker/src/main/java/org/apache/logging/log4j/docker/DockerLookup.java
@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.IOException;
+import java.net.URI;
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
@@ -41,32 +42,28 @@
 public class DockerLookup extends AbstractLookup {
 
     private static final Logger LOGGER = StatusLogger.getLogger();
-    private static final String DOCKER_URI = "DOCKER_URI";
     private static final String HTTP = "http";
     private final Container container;
 
     /**
      * Constructs a new instance.
      */
     public DockerLookup() {
-        String baseUri = System.getenv(DOCKER_URI);
-        if (baseUri == null) {
-            final PropertyEnvironment props = PropertyEnvironment.getGlobal();
-            baseUri = props.getStringProperty(DOCKER_URI);
-        }
+        final URI baseUri = PropertyEnvironment.getGlobal()
+                .getProperty(DockerProperties.class)
+                .uri();
         if (baseUri == null) {
             LOGGER.warn("No Docker URI provided. Docker information is unavailable");
             container = null;
             return;
         }
         Container current = null;
         try {
-            final URL url = new URL(baseUri + "/containers/json");
+            final URL url = baseUri.resolve("/containers/json").toURL();
             if (url.getProtocol().equals(HTTP)) {
                 final String macAddr = NetUtils.getMacAddressString();
                 final ObjectMapper objectMapper = new ObjectMapper();
-                final List<Container> containerList =
-                        objectMapper.readValue(url, new TypeReference<List<Container>>() {});
+                final List<Container> containerList = objectMapper.readValue(url, new TypeReference<>() {});
 
                 for (final Container container : containerList) {
                     if (macAddr != null && container.getNetworkSettings() != null) {

diff --git a/...gins/condition/ConditionalOnProperty.java → ...ogging/log4j/docker/DockerProperties.java b/...gins/condition/ConditionalOnProperty.java → ...ogging/log4j/docker/DockerProperties.java
@@ -14,25 +14,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.logging.log4j.plugins.condition;
+package org.apache.logging.log4j.docker;
 
-import java.lang.annotation.Documented;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
+import java.net.URI;
+import org.apache.logging.log4j.kit.env.Log4jProperty;
+import org.jspecify.annotations.Nullable;
 
 /**
- * Checks if a Log4j property is present or matches a specific non-empty value.
+ * Properties for the Docker lookup.
+ * @param uri
  */
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE, ElementType.METHOD})
-@Documented
-@Conditional(OnPropertyCondition.class)
-public @interface ConditionalOnProperty {
-    String name();
-
-    String value() default "";
-
-    boolean matchIfMissing() default false;
-}
+@Log4jProperty(name = "docker")
+public record DockerProperties(@Nullable URI uri) {}
diff --git a/...-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java b/...-flume-ng/src/main/java/org/apache/logging/log4j/flume/appender/FlumeEmbeddedManager.java
@@ -19,6 +19,7 @@
 import static org.apache.logging.log4j.util.Strings.toRootUpperCase;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.io.File;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.HashMap;
@@ -31,13 +32,11 @@
 import org.apache.logging.log4j.core.appender.ManagerFactory;
 import org.apache.logging.log4j.core.config.ConfigurationException;
 import org.apache.logging.log4j.core.config.Property;
-import org.apache.logging.log4j.kit.env.PropertyEnvironment;
 import org.apache.logging.log4j.util.Strings;
 
 public class FlumeEmbeddedManager extends AbstractFlumeManager {
 
-    private static final String FILE_SEP = PropertyEnvironment.getGlobal().getStringProperty("file.separator");
-
+    private static final String FILE_SEP = File.separator;
     private static final String IN_MEMORY = "InMemory";
 
     private static final FlumeManagerFactory FACTORY = new FlumeManagerFactory();

diff --git a/log4j-jndi/src/main/java/org/apache/logging/log4j/jndi/lookup/JndiLookup.java b/log4j-jndi/src/main/java/org/apache/logging/log4j/jndi/lookup/JndiLookup.java
@@ -26,18 +26,13 @@
 import org.apache.logging.log4j.core.lookup.Lookup;
 import org.apache.logging.log4j.jndi.JndiManager;
 import org.apache.logging.log4j.plugins.Plugin;
-import org.apache.logging.log4j.plugins.validation.constraints.RequiredProperty;
 import org.apache.logging.log4j.status.StatusLogger;
 
 /**
  * Looks up keys from JNDI resources.
  */
 @Lookup
 @Plugin("jndi")
-@RequiredProperty(
-        name = "jndi.enableLookup",
-        value = "true",
-        message = "JNDI must be enabled by setting jndi.enableLookup=true")
 public class JndiLookup extends AbstractLookup {
 
     private static final Logger LOGGER = StatusLogger.getLogger();
@@ -46,13 +41,13 @@ public class JndiLookup extends AbstractLookup {
     /** JNDI resource path prefix used in a J2EE container */
     static final String CONTAINER_JNDI_RESOURCE_PATH_PREFIX = "java:comp/env/";
 
+    private final boolean disabled;
+
     /**
-     * Constructs a new instance or throw IllegalStateException if this feature is disabled.
+     * Constructs a new instance.
      */
     public JndiLookup() {
-        if (!JndiManager.isJndiLookupEnabled()) {
-            throw new IllegalStateException("JNDI must be enabled by setting log4j2.enableJndiLookup=true");
-        }
+        this.disabled = !JndiManager.isJndiLookupEnabled();
     }
 
     /**
@@ -64,7 +59,7 @@ public JndiLookup() {
      */
     @Override
     public String lookup(final LogEvent event, final String key) {
-        if (key == null) {
+        if (disabled || key == null) {
             return null;
         }
         final String jndiName = convertJndiName(key);