Skip to content

Simplify file manager registry factory pattern #3968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from
+242 −614
Open

Simplify file manager registry factory pattern #3968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
790e756
Simplify file manager registry factory pattern
ppkarwasz Oct 26, 2025
a9ab255
fix: add reference to PR number
ppkarwasz Oct 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public boolean stop(final long timeout, final TimeUnit timeUnit) {
* @param <M> The Type of the Manager to be created.
* @param <T> The type of the Factory data.
* @return A Manager with the specified name and type.
* @throws IllegalStateException if the factory is unable to create the manager
*/
// @SuppressWarnings("resource"): this is a factory method, the resource is allocated and released elsewhere.
@SuppressWarnings("resource")
Expand Down
162 changes: 37 additions & 125 deletions log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@
*/
public class FileManager extends OutputStreamManager {

private static final FileManagerFactory FACTORY = new FileManagerFactory();

private final boolean isAppend;
private final boolean createOnDemand;
private final boolean isLocking;
Expand Down Expand Up @@ -194,7 +192,7 @@ protected FileManager(
* @param locking true if the file should be locked while writing, false otherwise.
* @param bufferedIo true if the contents should be buffered as they are written.
* @param createOnDemand true if you want to lazy-create the file (a.k.a. on-demand.)
* @param advertiseUri the URI to use when advertising the file
* @param advertiseURI the URI to use when advertising the file
* @param layout The layout
* @param bufferSize buffer size for buffered IO
* @param filePermissions File permissions
Expand All @@ -209,34 +207,52 @@ public static FileManager getFileManager(
boolean locking,
final boolean bufferedIo,
final boolean createOnDemand,
final String advertiseUri,
final String advertiseURI,
final Layout<? extends Serializable> layout,
final int bufferSize,
final String filePermissions,
final String fileOwner,
final String fileGroup,
final Configuration configuration) {

if (locking && bufferedIo) {
locking = false;
}
// Disable locking if buffered IO is enabled
boolean actualLocking = !bufferedIo && locking;
int actualBufferSize = bufferedIo ? bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE;
return narrow(
FileManager.class,
getManager(
fileName,
new FactoryData(
append,
locking,
bufferedIo,
bufferSize,
createOnDemand,
advertiseUri,
layout,
filePermissions,
fileOwner,
fileGroup,
configuration),
FACTORY));
(name, data) -> {
Objects.requireNonNull(name, "filename is missing");
final File file = new File(name);
try {
FileUtils.makeParentDirs(file);
final boolean writeHeader = !append || !file.exists();
final ByteBuffer byteBuffer = ByteBuffer.allocate(actualBufferSize);
final FileOutputStream fos = createOnDemand ? null : new FileOutputStream(file, append);
final FileManager fm = new FileManager(
data.getLoggerContext(),
name,
fos,
append,
actualLocking,
createOnDemand,
advertiseURI,
layout,
filePermissions,
fileOwner,
fileGroup,
writeHeader,
byteBuffer);
if (fos != null && fm.attributeViewEnabled) {
fm.defineAttributeView(file.toPath());
}
return fm;
} catch (final IOException ex) {
LOGGER.error("FileManager ({}): {}", name, ex);
}
return null;
},
new ConfigurationFactoryData(configuration)));
}

@Override
Expand Down Expand Up @@ -426,108 +442,4 @@ public Map<String, String> getContentFormat() {
result.put("fileURI", advertiseURI);
return result;
}

/**
* Factory Data.
*/
private static class FactoryData extends ConfigurationFactoryData {
private final boolean append;
private final boolean locking;
private final boolean bufferedIo;
private final int bufferSize;
private final boolean createOnDemand;
private final String advertiseURI;
private final Layout<? extends Serializable> layout;
private final String filePermissions;
private final String fileOwner;
private final String fileGroup;

/**
* Constructor.
* @param append Append status.
* @param locking Locking status.
* @param bufferedIo Buffering flag.
* @param bufferSize Buffer size.
* @param createOnDemand if you want to lazy-create the file (a.k.a. on-demand.)
* @param advertiseURI the URI to use when advertising the file
* @param layout The layout
* @param filePermissions File permissions
* @param fileOwner File owner
* @param fileGroup File group
* @param configuration the configuration
*/
public FactoryData(
final boolean append,
final boolean locking,
final boolean bufferedIo,
final int bufferSize,
final boolean createOnDemand,
final String advertiseURI,
final Layout<? extends Serializable> layout,
final String filePermissions,
final String fileOwner,
final String fileGroup,
final Configuration configuration) {
super(configuration);
this.append = append;
this.locking = locking;
this.bufferedIo = bufferedIo;
this.bufferSize = bufferSize;
this.createOnDemand = createOnDemand;
this.advertiseURI = advertiseURI;
this.layout = layout;
this.filePermissions = filePermissions;
this.fileOwner = fileOwner;
this.fileGroup = fileGroup;
}
}

/**
* Factory to create a FileManager.
*/
private static class FileManagerFactory implements ManagerFactory<FileManager, FactoryData> {

/**
* Creates a FileManager.
* @param name The name of the File.
* @param data The FactoryData
* @return The FileManager for the File.
*/
@Override
@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = "The destination file should be specified in the configuration file.")
public FileManager createManager(final String name, final FactoryData data) {
Objects.requireNonNull(name, "filename is missing");
final File file = new File(name);
try {
FileUtils.makeParentDirs(file);
final boolean writeHeader = !data.append || !file.exists();
final int actualSize = data.bufferedIo ? data.bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE;
final ByteBuffer byteBuffer = ByteBuffer.wrap(new byte[actualSize]);
final FileOutputStream fos = data.createOnDemand ? null : new FileOutputStream(file, data.append);
final FileManager fm = new FileManager(
data.getLoggerContext(),
name,
fos,
data.append,
data.locking,
data.createOnDemand,
data.advertiseURI,
data.layout,
data.filePermissions,
data.fileOwner,
data.fileGroup,
writeHeader,
byteBuffer);
if (fos != null && fm.attributeViewEnabled) {
fm.defineAttributeView(file.toPath());
}
return fm;
} catch (final IOException ex) {
LOGGER.error("FileManager (" + name + ") " + ex, ex);
}
return null;
}
}
}
85 changes: 31 additions & 54 deletions log4j-core/src/main/java/org/apache/logging/log4j/core/appender/MemoryMappedFileManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
package org.apache.logging.log4j.core.appender;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
Expand Down Expand Up @@ -58,7 +57,6 @@ public class MemoryMappedFileManager extends OutputStreamManager {
static final int DEFAULT_REGION_LENGTH = 32 * 1024 * 1024;

private static final int MAX_REMAP_COUNT = 10;
private static final MemoryMappedFileManagerFactory FACTORY = new MemoryMappedFileManagerFactory();
private static final double NANOS_PER_MILLISEC = 1000.0 * 1000.0;

private final boolean immediateFlush;
Expand Down Expand Up @@ -111,8 +109,37 @@ public static MemoryMappedFileManager getFileManager(
MemoryMappedFileManager.class,
getManager(
fileName,
new FactoryData(append, immediateFlush, regionLength, advertiseURI, layout),
FACTORY));
(name, ignored) -> {
final File file = new File(name);
if (!append) {
file.delete();
}

final boolean writeHeader = !append || !file.exists();
final OutputStream os = NullOutputStream.getInstance();
RandomAccessFile raf = null;
try {
FileUtils.makeParentDirs(file);
raf = new RandomAccessFile(name, "rw");
final long position = (append) ? raf.length() : 0;
raf.setLength(position + regionLength);
return new MemoryMappedFileManager(
raf,
name,
os,
immediateFlush,
position,
regionLength,
advertiseURI,
layout,
writeHeader);
} catch (final Exception ex) {
LOGGER.error("MemoryMappedFileManager (" + name + ") " + ex, ex);
Closer.closeSilently(raf);
}
return null;
},
(Void) null));
}

/**
Expand Down Expand Up @@ -341,54 +368,4 @@ public FactoryData(
this.layout = layout;
}
}

/**
* Factory to create a MemoryMappedFileManager.
*/
private static class MemoryMappedFileManagerFactory
implements ManagerFactory<MemoryMappedFileManager, FactoryData> {

/**
* Create a MemoryMappedFileManager.
*
* @param name The name of the File.
* @param data The FactoryData
* @return The MemoryMappedFileManager for the File.
*/
@SuppressWarnings("resource")
@Override
@SuppressFBWarnings(
value = "PATH_TRAVERSAL_IN",
justification = "The destination file should be specified in the configuration file.")
public MemoryMappedFileManager createManager(final String name, final FactoryData data) {
final File file = new File(name);
if (!data.append) {
file.delete();
}

final boolean writeHeader = !data.append || !file.exists();
final OutputStream os = NullOutputStream.getInstance();
RandomAccessFile raf = null;
try {
FileUtils.makeParentDirs(file);
raf = new RandomAccessFile(name, "rw");
final long position = (data.append) ? raf.length() : 0;
raf.setLength(position + data.regionLength);
return new MemoryMappedFileManager(
raf,
name,
os,
data.immediateFlush,
position,
data.regionLength,
data.advertiseURI,
data.layout,
writeHeader);
} catch (final Exception ex) {
LOGGER.error("MemoryMappedFileManager (" + name + ") " + ex, ex);
Closer.closeSilently(raf);
}
return null;
}
}
}
Loading