Skip to content

Move the remaining log4j 1.x API uses to log4j2 - #539

Open
NicoPiel wants to merge 4 commits into
OpenIntegrationEngine:mainfrom
NicoPiel:modernise/log4j1-api
Open

Move the remaining log4j 1.x API uses to log4j2#539
NicoPiel wants to merge 4 commits into
OpenIntegrationEngine:mainfrom
NicoPiel:modernise/log4j1-api

Conversation

@NicoPiel

@NicoPiel NicoPiel commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

What

Moves the last Java uses of the log4j 1.x API to log4j2:

  • DefaultDebugUsageController now uses LogManager/Logger like the other controllers.
  • ExtensionStatuses and ExtensionStatusProvider look up org.apache.logging.log4j.LogManager reflectively instead of org.apache.log4j.Logger.

Why

These were the only Java callers of the 1.2 bridge API. The bridge itself stays, because the generated transformer script still exposes org.apache.log4j.Logger to user scripts.

Reviewer notes

  • The extprops classes are packed into the launcher jar, which has no log4j on its classpath, so they keep the reflective lookup through the context classloader. That's the same lookup and LoggerWrapper usage MirthLauncher already does.
  • Verified with ./gradlew :server:compileJava and DebugUsageControllerTest. No automated test covers the extprops path.

🤖 Generated with Claude Code

@NicoPiel
Use the log4j2 API in DefaultDebugUsageController
171049d 
Signed-off-by: Nico Piel <nico.piel@hotmail.de>
@NicoPiel
Load the log4j2 LogManager reflectively in extprops
495f775 
The launcher jar has no log4j on its classpath, so the logger is still looked up through the context classloader, the same way MirthLauncher already does it.

Signed-off-by: Nico Piel <nico.piel@hotmail.de>
@github-actions

github-actions Bot commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Results

128 files   - 2  128 suites   - 2   3m 48s ⏱️ +37s
788 tests  - 1  788 ✅  - 1  0 💤 ±0  0 ❌ ±0 
914 runs   - 1  908 ✅  - 1  6 💤 ±0  0 ❌ ±0 

Results for commit dd012af. ± Comparison against base commit 4589238.

This pull request removes 3 and adds 2 tests. Note that renamed tests count towards both. 
com.mirth.connect.connectors.ws.WebServiceConnectorServletTest ‑ testCommonsSSL
com.mirth.connect.donkey.server.data.jdbc.OracleJdbcDaoTest ‑ testBatchInsertMessageContentKeepsStatementOpenUntilExecuted
com.mirth.connect.donkey.server.data.jdbc.OracleJdbcDaoTest ‑ testFailedBatchInsertClearsAndClosesTheStatement

com.mirth.connect.donkey.server.data.jdbc.JdbcDaoTest ‑ insertMessageContentBatchBatchesEveryRowThenExecutesOnce
com.mirth.connect.donkey.server.data.jdbc.JdbcDaoTest ‑ insertMessageContentBatchClosesTheStatementWhenBindingFails

♻️ This comment has been updated with latest results.

gibson9583
gibson9583 previously approved these changes Oct 30, 2026
View reviewed changes

@gibson9583 gibson9583 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe to merge.

mgaffigan
mgaffigan previously approved these changes Oct 30, 2026
View reviewed changes
@mgaffigan

mgaffigan commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Let's kill the bridge, too. We don't want to break user scripts unnecessarily, but this one seems like it can be compensated by a plugin - and the scripts have had a decade or two to update.

@NicoPiel
Drop the log4j 1.2 bridge
e831ac2 
Nothing in the engine calls the log4j 1.x API any more, so remove
log4j-1.2-api from the distribution. slf4j-log4j12 bound slf4j to the
log4j 1.x classes the bridge provided, so replace it with
log4j-slf4j-impl. The transformer script now resets its logger through
the log4j2 LogManager.

The vendored not-going-to-be-commons-ssl jar called log4j 1.x directly
and was only kept for SoapUI, which the engine no longer uses. Remove
it together with its regression test and the leftover SoapUI log4j
config.

Signed-off-by: Nico Piel <nico.piel@hotmail.de>
@NicoPiel
NicoPiel dismissed stale reviews from mgaffigan and gibson9583 via e831ac2 October 30, 2026 15:50
@NicoPiel
Route MyBatis logging through commons-logging
dd012af 
MyBatis 3.1.1 has no log4j2 adapter, and its log4j adapter needs the
log4j 1.x classes the bridge used to provide, so the server could not
start. commons-logging hands MyBatis logging to the log4j2 API.

Signed-off-by: Nico Piel <nico.piel@hotmail.de>
tonygermano
tonygermano reviewed Oct 31, 2026
View reviewed changes
Comment thread server/src/main/java/com/mirth/connect/server/util/SqlConfig.java
private void init() {
try {
LogFactory.useLog4JLogging();
LogFactory.useCommonsLogging();

@tonygermano tonygermano Oct 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this only needed because the MyBatis upgrade hasn't merged yet?

@NicoPiel NicoPiel Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. MyBatis 3.1.1 has no log4j2 adapter (it arrived in 3.2), and its log4j adapter needs the log4j 1.x classes this PR removes, so commons-logging is the only route to log4j2 for now. Once #545 (MyBatis 3.5.19) lands, this can become LogFactory.useLog4J2Logging(); whichever PR merges second will make that switch.

🤖 Addressed by Claude Code

@pacmano1

pacmano1 commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

This also fixes the CLI's exportmessages, which is broken in 4.7.0. The CLI prints "Exporting messages to file", then dies with NoClassDefFoundError: org/apache/log4j/Level (Velocity's SLF4J init, via ValueReplacer), writes nothing, and exits 0. cli-lib ships slf4j-log4j12 without the log4j 1.2 bridge.

Built dd012af and ran three CLIs against its server, same channel, 3 messages:

  • this branch's CLI: 3 exported
  • stock 4.7.0 CLI: the NoClassDefFoundError, nothing written, exit 0
  • 4.7.0 CLI with only slf4j-log4j12 swapped for log4j-slf4j-impl-2.26.1: 3 exported

Might be worth a line in the description, since it's a user-facing fix.

Co-authored with Claude Code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonygermano tonygermano tonygermano left review comments
@gibson9583 gibson9583 gibson9583 left review comments
@mgaffigan mgaffigan mgaffigan left review comments
@kayyagari kayyagari Awaiting requested review from kayyagari kayyagari is a code owner automatically assigned from OpenIntegrationEngine/maintainers
@ssrowe ssrowe Awaiting requested review from ssrowe ssrowe is a code owner automatically assigned from OpenIntegrationEngine/maintainers
@kpalang kpalang Awaiting requested review from kpalang kpalang is a code owner automatically assigned from OpenIntegrationEngine/maintainers
@jonbartels jonbartels Awaiting requested review from jonbartels jonbartels is a code owner automatically assigned from OpenIntegrationEngine/maintainers
@pacmano1 pacmano1 Awaiting requested review from pacmano1 pacmano1 is a code owner automatically assigned from OpenIntegrationEngine/maintainers

At least 3 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@NicoPiel @mgaffigan @pacmano1 @gibson9583 @tonygermano