This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch 2.24.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 755497d3715efe80c543525a0fec3e7171c88dab Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Sep 23 22:45:30 2024 +0200 Revert "Single constructor for `ConfigurationSource`" This reverts commit 35973f73ac4f9527a8a46b4b9a20246a6912eaa5. --- .../logging/log4j/core/util/WatchManagerTest.java | 4 +- .../log4j/core/config/AbstractConfiguration.java | 15 ++-- .../log4j/core/config/ConfigurationSource.java | 79 ++++++++++++---------- .../org/apache/logging/log4j/core/util/Loader.java | 5 +- .../org/apache/logging/log4j/core/util/Source.java | 45 +++++------- 5 files changed, 69 insertions(+), 79 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java index 9e2b4e92ce..49269d7134 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java @@ -36,6 +36,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import org.apache.logging.log4j.core.config.ConfigurationScheduler; +import org.apache.logging.log4j.core.config.ConfigurationSource; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -143,10 +144,9 @@ class WatchManagerTest { */ @Test void testWatchManagerCallsWatcher() { - Source source = mock(Source.class); Watcher watcher = mock(Watcher.class); when(watcher.isModified()).thenReturn(false); - watchManager.watch(source, watcher); + watchManager.watch(new Source(ConfigurationSource.NULL_SOURCE), watcher); verify(watcher, timeout(2000)).isModified(); verify(watcher, never()).modified(); when(watcher.isModified()).thenReturn(true); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index 3b301bde4d..4b160f98ef 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java @@ -17,12 +17,10 @@ package org.apache.logging.log4j.core.config; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.Serializable; import java.lang.ref.WeakReference; -import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -281,10 +279,9 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement if (configSource != null && (configSource.getFile() != null || configSource.getURL() != null)) { if (monitorIntervalSeconds > 0) { watchManager.setIntervalSeconds(monitorIntervalSeconds); - File file = configSource.getFile(); - if (file != null) { - final Source cfgSource = new Source(file); - final long lastModified = file.lastModified(); + if (configSource.getFile() != null) { + final Source cfgSource = new Source(configSource); + final long lastModified = configSource.getFile().lastModified(); final ConfigurationFileWatcher watcher = new ConfigurationFileWatcher(this, reconfigurable, listeners, lastModified); watchManager.watch(cfgSource, watcher); @@ -300,10 +297,8 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement } private void monitorSource(final Reconfigurable reconfigurable, final ConfigurationSource configSource) { - URI uri = configSource.getURI(); - if (uri != null && configSource.getLastModified() > 0) { - File file = configSource.getFile(); - final Source cfgSource = file != null ? new Source(file) : new Source(uri); + if (configSource.getLastModified() > 0) { + final Source cfgSource = new Source(configSource); final Watcher watcher = WatcherFactory.getInstance(pluginPackages) .newWatcher(cfgSource, this, reconfigurable, listeners, configSource.getLastModified()); if (watcher != null) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java index 1fd47fa975..ff443b08f3 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java @@ -16,8 +16,6 @@ */ package org.apache.logging.log4j.core.config; -import static java.util.Objects.requireNonNull; - import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -34,6 +32,7 @@ import java.net.URL; import java.net.URLConnection; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Objects; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.net.UrlConnectionFactory; import org.apache.logging.log4j.core.util.FileUtils; @@ -78,7 +77,16 @@ public class ConfigurationSource { * @param file the file where the input stream originated */ public ConfigurationSource(final InputStream stream, final File file) { - this(null, new Source(file), stream, getLastModified(file.toPath())); + this.stream = Objects.requireNonNull(stream, "stream is null"); + this.data = null; + this.source = new Source(file); + long modified = 0; + try { + modified = file.lastModified(); + } catch (Exception ex) { + // There is a problem with the file. It will be handled somewhere else. + } + this.currentLastModified = this.initialLastModified = modified; } /** @@ -89,16 +97,16 @@ public class ConfigurationSource { * @param path the path where the input stream originated. */ public ConfigurationSource(final InputStream stream, final Path path) { - this(null, new Source(path), stream, getLastModified(path)); - } - - private static long getLastModified(Path path) { + this.stream = Objects.requireNonNull(stream, "stream is null"); + this.data = null; + this.source = new Source(path); + long modified = 0; try { - return Files.getLastModifiedTime(path).toMillis(); - } catch (Exception ignored) { + modified = Files.getLastModifiedTime(path).toMillis(); + } catch (Exception ex) { // There is a problem with the file. It will be handled somewhere else. } - return 0L; + this.currentLastModified = this.initialLastModified = modified; } /** @@ -120,8 +128,11 @@ public class ConfigurationSource { * @param url the URL where the input stream originated * @param lastModified when the source was last modified. */ - public ConfigurationSource(InputStream stream, final URL url, final long lastModified) { - this(null, new Source(url), stream, lastModified); + public ConfigurationSource(final InputStream stream, final URL url, final long lastModified) { + this.stream = Objects.requireNonNull(stream, "stream is null"); + this.data = null; + this.currentLastModified = this.initialLastModified = lastModified; + this.source = new Source(url); } /** @@ -131,7 +142,7 @@ public class ConfigurationSource { * @param stream the input stream, the caller is responsible for closing this resource. * @throws IOException if an exception occurred reading from the specified stream */ - public ConfigurationSource(InputStream stream) throws IOException { + public ConfigurationSource(final InputStream stream) throws IOException { this(toByteArray(stream), null, 0); } @@ -142,26 +153,19 @@ public class ConfigurationSource { * @param data data from the source * @param lastModified when the source was last modified. */ - public ConfigurationSource(Source source, byte[] data, long lastModified) { - this(data, requireNonNull(source, "source is null"), lastModified); - } - - private ConfigurationSource(byte[] data, /*@Nullable*/ Source source, long lastModified) { - this(requireNonNull(data, "data is null"), source, new ByteArrayInputStream(data), lastModified); + public ConfigurationSource(final Source source, final byte[] data, final long lastModified) { + Objects.requireNonNull(source, "source is null"); + this.data = Objects.requireNonNull(data, "data is null"); + this.stream = new ByteArrayInputStream(data); + this.currentLastModified = this.initialLastModified = lastModified; + this.source = source; } - /** - * @throws NullPointerException if both {@code stream} and {@code data} are {@code null}. - */ - private ConfigurationSource( - byte /*@Nullable*/[] data, /*@Nullable*/ Source source, InputStream stream, long lastModified) { - if (data == null && source == null) { - throw new NullPointerException("both `data` and `source` are null"); - } - this.stream = stream; - this.data = data; - this.source = source; + private ConfigurationSource(final byte[] data, final URL url, final long lastModified) { + this.data = Objects.requireNonNull(data, "data is null"); + this.stream = new ByteArrayInputStream(data); this.currentLastModified = this.initialLastModified = lastModified; + this.source = url == null ? null : new Source(url); } /** @@ -194,6 +198,10 @@ public class ConfigurationSource { return source == null ? null : source.getFile(); } + private boolean isLocation() { + return source != null && source.getLocation() != null; + } + /** * Returns the configuration source URL, or {@code null} if this configuration source is based on a file or has * neither a file nor an URL. @@ -279,24 +287,23 @@ public class ConfigurationSource { URL url = getURL(); if (url != null && data != null) { // Creates a ConfigurationSource without accessing the URL since the data was provided. - return new ConfigurationSource(data, new Source(url), currentLastModified); + return new ConfigurationSource(data, url, currentLastModified); } URI uri = getURI(); if (uri != null) { return fromUri(uri); } - return data == null ? null : new ConfigurationSource(data, null, currentLastModified); + return data != null ? new ConfigurationSource(data, null, currentLastModified) : null; } @Override public String toString() { - if (source != null) { - return source.getLocation(); + if (isLocation()) { + return getLocation(); } if (this == NULL_SOURCE) { return "NULL_SOURCE"; } - byte[] data = this.data; final int length = data == null ? -1 : data.length; return "stream (" + length + " bytes, unknown location)"; } @@ -304,7 +311,7 @@ public class ConfigurationSource { /** * Loads the configuration from a URI. * @param configLocation A URI representing the location of the configuration. - * @return The ConfigurationSource for the configuration or {@code null}. + * @return The ConfigurationSource for the configuration. */ public static /*@Nullable*/ ConfigurationSource fromUri(final URI configLocation) { final File configFile = FileUtils.fileFromUri(configLocation); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Loader.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Loader.java index 64ee6be359..b4e035c9fb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Loader.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Loader.java @@ -23,7 +23,6 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.LoaderUtil; import org.apache.logging.log4j.util.PropertiesUtil; -import org.jspecify.annotations.Nullable; /** * Load resources (or images) from various sources. @@ -85,9 +84,9 @@ public final class Loader { * </ol> * @param resource The resource to load. * @param defaultLoader The default ClassLoader. - * @return A URL to the resource or {@code null}. + * @return A URL to the resource. */ - public static @Nullable URL getResource(final String resource, final ClassLoader defaultLoader) { + public static URL getResource(final String resource, final ClassLoader defaultLoader) { try { ClassLoader classLoader = getThreadContextClassLoader(); if (classLoader != null) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java index 5a79ab8b9a..580d3b73d4 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java @@ -16,8 +16,6 @@ */ package org.apache.logging.log4j.core.util; -import static java.util.Objects.requireNonNull; - import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.File; import java.io.IOException; @@ -32,13 +30,10 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.ConfigurationSource; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Strings; -import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.Nullable; /** * Represents the source for the logging configuration as an immutable object. */ -@NullMarked public class Source { private static final Logger LOGGER = StatusLogger.getLogger(); @@ -50,9 +45,9 @@ public class Source { } } - private static @Nullable File toFile(Path path) { + private static File toFile(final Path path) { try { - return requireNonNull(path, "path").toFile(); + return Objects.requireNonNull(path, "path").toFile(); } catch (final UnsupportedOperationException e) { return null; } @@ -62,9 +57,9 @@ public class Source { @SuppressFBWarnings( value = "PATH_TRAVERSAL_IN", justification = "The URI should be specified in a configuration file.") - private static @Nullable File toFile(URI uri) { + private static File toFile(final URI uri) { try { - final String scheme = requireNonNull(uri, "uri").getScheme(); + final String scheme = Objects.requireNonNull(uri, "uri").getScheme(); if (Strings.isBlank(scheme) || scheme.equals("file")) { return new File(uri.getPath()); } else { @@ -72,20 +67,20 @@ public class Source { return null; } } catch (final Exception e) { - LOGGER.debug("uri is malformed: " + uri); + LOGGER.debug("uri is malformed: " + uri.toString()); return null; } } private static URI toURI(final URL url) { try { - return requireNonNull(url, "url").toURI(); + return Objects.requireNonNull(url, "url").toURI(); } catch (final URISyntaxException e) { throw new IllegalArgumentException(e); } } - private final @Nullable File file; + private final File file; private final URI uri; private final String location; @@ -93,23 +88,21 @@ public class Source { * Constructs a Source from a ConfigurationSource. * * @param source The ConfigurationSource. - * @throws NullPointerException if {@code source} is {@code null}. */ public Source(final ConfigurationSource source) { this.file = source.getFile(); - this.uri = requireNonNull(source.getURI()); - this.location = requireNonNull(source.getLocation()); + this.uri = source.getURI(); + this.location = source.getLocation(); } /** * Constructs a new {@code Source} with the specified file. * file. * - * @param file the file where the input stream originated. - * @throws NullPointerException if {@code file} is {@code null}. + * @param file the file where the input stream originated */ public Source(final File file) { - this.file = requireNonNull(file, "file"); + this.file = Objects.requireNonNull(file, "file"); this.location = normalize(file); this.uri = file.toURI(); } @@ -118,10 +111,9 @@ public class Source { * Constructs a new {@code Source} from the specified Path. * * @param path the Path where the input stream originated - * @throws NullPointerException if {@code path} is {@code null}. */ public Source(final Path path) { - final Path normPath = requireNonNull(path, "path").normalize(); + final Path normPath = Objects.requireNonNull(path, "path").normalize(); this.file = toFile(normPath); this.uri = normPath.toUri(); this.location = normPath.toString(); @@ -131,10 +123,9 @@ public class Source { * Constructs a new {@code Source} from the specified URI. * * @param uri the URI where the input stream originated - * @throws NullPointerException if {@code uri} is {@code null}. */ public Source(final URI uri) { - final URI normUri = requireNonNull(uri, "uri").normalize(); + final URI normUri = Objects.requireNonNull(uri, "uri").normalize(); this.uri = normUri; this.location = normUri.toString(); this.file = toFile(normUri); @@ -144,12 +135,11 @@ public class Source { * Constructs a new {@code Source} from the specified URI. * * @param uri the URI where the input stream originated - * @param ignored Not used. + * @param lastModified Not used. * @deprecated Use {@link Source#Source(URI)}. - * @throws NullPointerException if {@code uri} is {@code null}. */ @Deprecated - public Source(URI uri, long ignored) { + public Source(final URI uri, final long lastModified) { this(uri); } @@ -157,7 +147,6 @@ public class Source { * Constructs a new {@code Source} from the specified URL. * * @param url the URL where the input stream originated - * @throws NullPointerException if this URL is {@code null}. * @throws IllegalArgumentException if this URL is not formatted strictly according to RFC2396 and cannot be * converted to a URI. */ @@ -185,7 +174,7 @@ public class Source { * * @return the configuration source file, or {@code null} */ - public @Nullable File getFile() { + public File getFile() { return file; } @@ -208,7 +197,7 @@ public class Source { value = "PATH_TRAVERSAL_IN", justification = "The `file`, `uri` and `location` fields come from Log4j properties.") public Path getPath() { - return file != null ? file.toPath() : Paths.get(uri); + return file != null ? file.toPath() : uri != null ? Paths.get(uri) : Paths.get(location); } /**
