Honor ConfigurationFactory active state - #4199
Conversation
|
@DragonFSKY I've just taken a quick look at your PR and noticed there are multiple new test cases. Since it touches multiple files, please give us a little time. We will review it ASAP! |
ramanathan1504
left a comment
There was a problem hiding this comment.
Result: with jackson-dataformat-yaml absent, getSupportedTypes() still returns the YAML suffixes, so today a log4j2.yaml match logs that ERROR naming the file and the likely cause. After this change the factory is skipped before that point and nothing is logged. Please log before the continue.
|
Thanks for catching this! I've fixed it. Please let me know if anything else needs adjusting. |
d8f4a05 to
7b7c20b
Compare
7b7c20b to
6b8ec39
Compare
|
@DragonFSKY don't use force push.it will completely clear the old review..Thanks |
|
Thanks for the reminder, and sorry about that. I'll avoid force-pushing and keep any further review updates as separate commits. Since I had already squashed the branch before seeing your note, would you prefer that I leave it as is, or split the current change into two commits for easier review? I understand that this would require one more force-push, so I won't change the history unless you prefer it. |
|
Thanks for the review. I’ve moved the active-state checks to the factory loops and applied the suggested diagnostic and changelog updates. |
Fixes #3896.
This change makes
ConfigurationFactoryactive-state handling consistent:null, and aggregate lookup does not invoke them to create configurations.isActive()implementations, while Log4j 1.x compatibility factories track the compatibility property dynamically.Tests cover aggregate lookup paths, inactive-factory diagnostics and fallback, dynamic activation, overridden active state, and missing JSON/YAML dependencies. Existing Log4j 1.x configuration tests explicitly enable compatibility mode where required, and the affected OSGi package versions reflect the protected overrides.
Checklist
2.xbranch if you are targeting Log4j 2; usemainotherwise./mvnw -pl log4j-core-test,log4j-1.2-api -am verifysucceeds (the build instructions)src/changelog/.2.x.xdirectory