Simplify file manager registry factory pattern (#3968)

* Simplify file manager registry factory pattern

The current factory pattern used with the `AbstractManager` registry is unnecessarily complex. It relies on:

* **Stateless factory classes** to create managers of each type,
* **Separate `FactoryData` parameter objects** to pass construction data

#### What this PR does

This PR simplifies the registry factory mechanism by:

* Replacing the factory classes with **inline lambda factories**
* Allowing **stateful factory lambdas**, removing boilerplate `FactoryData` classes in most cases
* Keeping factory logic **localized and easier to follow** within the registry call site

This is a **pure refactoring**, it does not change behavior. It improves code readability and maintainability by reducing indirection and eliminating unnecessary types.

#### Notes

`FactoryData` parameter objects are still used where necessary:

* **`RollingFileManager`** and **`RollingRandomAccessFileManager`** still require parameter objects since they update state on each retrieval, not just during initialization.
* Other managers still pass `Configuration` through `ConfigurationFactoryData` to access `getLoggerContext()`, which remains useful.

* fix: add reference to PR number

* Apply suggestions from code review

Co-authored-by: Volkan Yazıcı <[email protected]>

---------

Co-authored-by: Volkan Yazıcı <[email protected]>

Author: ppkarwasz

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java

@@ -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")

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java

@@ -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;
@@ -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
@@ -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
@@ -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;
-        }
-    }
 }

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/MemoryMappedFileManager.java

@@ -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;
@@ -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;
@@ -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));
     }
 
     /**
@@ -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;
-        }
-    }
 }