You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2025/10/26 11:15:26 UTC
[PR] Simplify file manager registry factory pattern [logging-log4j2]
ppkarwasz opened a new pull request, #3968:
URL: https://github.com/apache/logging-log4j2/pull/3968
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.
## Checklist
### ๐งช Tests (select one)
- [ ] I have added or updated tests to cover my changes.
- [x] No additional tests are needed for this change.
### ๐ Changelog (select one)
- [x] I added a changelog entry in `src/changelog/.2.x.x`. (See [Changelog Entry File Guide](https://logging.apache.org/log4j/tools/log4j-changelog.html#changelog-entry-file)).
- [ ] This is a trivial change and does not require a changelog entry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify file manager registry factory pattern [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #3968:
URL: https://github.com/apache/logging-log4j2/pull/3968#issuecomment-3520744498
> Ideally `getFileManager()` should pass a DTO instead of 3 dozen arguments, but that is a thicker refactoring.
This is exacly what I tried to eliminate here: a `private` DTO (`FactoryData`) that is used in a single method is more confusing than useful. Configuration data ends up being packed and unpacked multiple times:
1. It is packed into a `FileAppender.Builder`,
2. It is unpacked and passed to `FileManager.getFileManager()`,
3. It is packed into a `FileManager.FactoryData`,
4. It is unpacked again by `FileManagerFactory`,
5. It is finally used be the `FileManager` constructor.
All along this path, the data is validated multiple times, defaults are applied and also some side-effects (creation of parent directory) appear.
If `FileManager` was a class carefully designed to be reusable, then I agree: a DTO (possibly a Java record) as only parameter to `getFileManager()` would be useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify file manager registry factory pattern [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #3968:
URL: https://github.com/apache/logging-log4j2/pull/3968#discussion_r2509475316
##########
src/changelog/.2.x.x/3968_refactor_manager_factories.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="changed">
+ <issue id="3968" link="https://github.com/apache/logging-log4j2/pull/3968"/>
+ <description format="asciidoc">
+ Simplify file manager registry factory methods by removing unnecessary boilerplate code.
Review Comment:
Shall we remove the negative expression `unnecessary boilerplate code`?
```suggestion
Simplify file manager registry factory methods
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify file manager registry factory pattern [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #3968:
URL: https://github.com/apache/logging-log4j2/pull/3968
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org