mirror of
https://github.com/apache/struts.git
synced 2026-08-05 14:47:09 +00:00
WW-5620 Standardize logging on Log4j2 (#1794)
* WW-5620 docs: add Log4j2 logging standardization design spec Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 docs: add Log4j2 logging standardization implementation plan Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Migrate FinalizableReferenceQueue to Log4j2 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Migrate AbstractDefaultToStringRenderable to Log4j2 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Remove unused injectable j.u.l.Logger DI factory from ContainerBuilder Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Remove dead first-party SLF4J dependency declarations Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Lukasz Lenart
@@ -316,18 +316,6 @@
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
<!-- SLF4J support -->
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
<optional>true</optional>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-simple</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
<dependency>
|
||||
<groupId>commons-validator</groupId>
|
||||
<artifactId>commons-validator</artifactId>
|
||||
|
||||
@@ -16,14 +16,12 @@
|
||||
|
||||
package org.apache.struts2.inject;
|
||||
|
||||
import java.lang.reflect.Member;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.HashMap;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.logging.Logger;
|
||||
|
||||
/**
|
||||
* Builds a dependency injection {@link Container}. The combination of
|
||||
@@ -34,7 +32,6 @@ import java.util.logging.Logger;
|
||||
*
|
||||
* <ul>
|
||||
* <li>Injects the current {@link Container}.
|
||||
* <li>Injects the {@link Logger} for the injected member's declaring class.
|
||||
* </ul>
|
||||
*
|
||||
* @author crazybob@google.com (Bob Lee)
|
||||
@@ -60,29 +57,12 @@ public final class ContainerBuilder {
|
||||
}
|
||||
};
|
||||
|
||||
private static final InternalFactory<Logger> LOGGER_FACTORY =
|
||||
new InternalFactory<>() {
|
||||
public Logger create(InternalContext context) {
|
||||
Member member = context.getExternalContext().getMember();
|
||||
return member == null ? Logger.getAnonymousLogger()
|
||||
: Logger.getLogger(member.getDeclaringClass().getName());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Class<? extends Logger> type() {
|
||||
return Logger.class;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Constructs a new builder.
|
||||
*/
|
||||
public ContainerBuilder() {
|
||||
// In the current container as the default Container implementation.
|
||||
factories.put(Key.newInstance(Container.class, Container.DEFAULT_NAME), CONTAINER_FACTORY);
|
||||
|
||||
// Inject the logger for the injected member's declaring class.
|
||||
factories.put(Key.newInstance(Logger.class, Container.DEFAULT_NAME), LOGGER_FACTORY);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -19,8 +19,8 @@ package org.apache.struts2.inject.util;
|
||||
import java.lang.ref.Reference;
|
||||
import java.lang.ref.ReferenceQueue;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
|
||||
/**
|
||||
* Starts a background thread that cleans up after reclaimed referents.
|
||||
@@ -29,8 +29,7 @@ import java.util.logging.Logger;
|
||||
*/
|
||||
public class FinalizableReferenceQueue extends ReferenceQueue<Object> {
|
||||
|
||||
private static final Logger logger =
|
||||
Logger.getLogger(FinalizableReferenceQueue.class.getName());
|
||||
private static final Logger LOG = LogManager.getLogger(FinalizableReferenceQueue.class);
|
||||
|
||||
private final AtomicReference<Thread> cleanupThread = new AtomicReference<>();
|
||||
|
||||
@@ -45,7 +44,7 @@ public class FinalizableReferenceQueue extends ReferenceQueue<Object> {
|
||||
}
|
||||
|
||||
void deliverBadNews(Throwable t) {
|
||||
logger.log(Level.SEVERE, "Error cleaning up after reference.", t);
|
||||
LOG.error("Error cleaning up after reference.", t);
|
||||
}
|
||||
|
||||
void start() {
|
||||
|
||||
@@ -0,0 +1,345 @@
|
||||
# WW-5620 Log4j2 Logging Standardization Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Migrate the three remaining legacy-logging source files to Log4j2 and remove the now-dead first-party SLF4J dependency declarations.
|
||||
|
||||
**Architecture:** Three independent, mostly mechanical source edits (two class-own logger swaps, one dead-DI-feature removal), followed by a pom cleanup gated on a `mvn dependency:tree` check. No functional or security behavior changes.
|
||||
|
||||
**Tech Stack:** Java, Log4j2 (`org.apache.logging.log4j`), Maven. Core tests are JUnit 4 + AssertJ + Mockito in this repo.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Fix version: 7.3.0. Target ticket: WW-5620. Every commit message MUST be prefixed `WW-5620`.
|
||||
- Logger convention: `private static final Logger LOG = LogManager.getLogger(<Class>.class);` for static class loggers (see `ActionSupport`, `ObjectFactory`). Preserve the existing field style where a file already uses an instance logger.
|
||||
- No new pom dependency may be added. `log4j-api` is already available (directly in core, transitively in tiles via `struts2-core`).
|
||||
- Do NOT touch the runtime bridges `log4j-jcl`, `log4j-slf4j-impl`, or `commons-logging` — they route third-party logging and are out of scope.
|
||||
- Build/test commands: `mvn test -DskipAssembly -pl core` and `mvn test -DskipAssembly -pl plugins/tiles`.
|
||||
- Reference spec: `docs/superpowers/specs/2026-07-20-WW-5620-log4j2-logging-standardization-design.md`.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Modify: `core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java` — swap `java.util.logging` → Log4j2.
|
||||
- Modify: `plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java` — swap SLF4J → Log4j2.
|
||||
- Modify: `core/src/main/java/org/apache/struts2/inject/ContainerBuilder.java` — remove the injectable `java.util.logging.Logger` DI factory.
|
||||
- Modify: `core/pom.xml`, `parent/pom.xml`, `pom.xml` — remove dead first-party SLF4J declarations.
|
||||
|
||||
No test files are created: files 1–2 are behavior-preserving; file 3 removes dead, untested code. Verification is via existing suites, `dependency:tree`, and grep.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Migrate `FinalizableReferenceQueue` to Log4j2
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: nothing from other tasks.
|
||||
- Produces: nothing consumed by other tasks (internal logger only).
|
||||
|
||||
- [ ] **Step 1: Confirm current state**
|
||||
|
||||
Run: `grep -n "logging\|logger\|Level" core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java`
|
||||
Expected: lines showing `import java.util.logging.Level;`, `import java.util.logging.Logger;`, the `logger` field (`Logger.getLogger(...getName())`), and `logger.log(Level.SEVERE, "Error cleaning up after reference.", t);`.
|
||||
|
||||
- [ ] **Step 2: Replace the imports**
|
||||
|
||||
Replace:
|
||||
```java
|
||||
import java.util.logging.Level;
|
||||
import java.util.logging.Logger;
|
||||
```
|
||||
with:
|
||||
```java
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
```
|
||||
(Keep these in import-order position consistent with the file; the surrounding `java.lang.ref.*` / `java.util.concurrent.*` imports stay.)
|
||||
|
||||
- [ ] **Step 3: Replace the logger field**
|
||||
|
||||
Replace:
|
||||
```java
|
||||
private static final Logger logger =
|
||||
Logger.getLogger(FinalizableReferenceQueue.class.getName());
|
||||
```
|
||||
with:
|
||||
```java
|
||||
private static final Logger LOG = LogManager.getLogger(FinalizableReferenceQueue.class);
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Replace the log call**
|
||||
|
||||
Replace:
|
||||
```java
|
||||
logger.log(Level.SEVERE, "Error cleaning up after reference.", t);
|
||||
```
|
||||
with:
|
||||
```java
|
||||
LOG.error("Error cleaning up after reference.", t);
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Verify no leftover legacy references**
|
||||
|
||||
Run: `grep -n "java.util.logging\|Level\.\|logger\b" core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java`
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 6: Compile core**
|
||||
|
||||
Run: `mvn -q -o compile -pl core -DskipAssembly` (drop `-o` if the local repo needs downloads)
|
||||
Expected: BUILD SUCCESS.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java
|
||||
git commit -m "WW-5620 Migrate FinalizableReferenceQueue to Log4j2
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Migrate `AbstractDefaultToStringRenderable` to Log4j2
|
||||
|
||||
**Files:**
|
||||
- Modify: `plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: nothing from other tasks.
|
||||
- Produces: nothing consumed by other tasks (internal instance logger only).
|
||||
|
||||
- [ ] **Step 1: Confirm current state**
|
||||
|
||||
Run: `grep -n "slf4j\|Logger\|log\." plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java`
|
||||
Expected: `import org.slf4j.Logger;`, `import org.slf4j.LoggerFactory;`, `private final Logger log = LoggerFactory.getLogger(getClass());`, and `log.error("Error when closing a StringWriter, the impossible happened!", e);`.
|
||||
|
||||
- [ ] **Step 2: Replace the imports**
|
||||
|
||||
Replace:
|
||||
```java
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
```
|
||||
with:
|
||||
```java
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
```
|
||||
(Keep import ordering consistent with the file; `org.apache.logging...` sorts near the other `org.apache.*` imports.)
|
||||
|
||||
- [ ] **Step 3: Replace the logger field factory call**
|
||||
|
||||
Replace:
|
||||
```java
|
||||
private final Logger log = LoggerFactory.getLogger(getClass());
|
||||
```
|
||||
with:
|
||||
```java
|
||||
private final Logger log = LogManager.getLogger(getClass());
|
||||
```
|
||||
(The `log.error(...)` call is unchanged — the Log4j2 `Logger.error(String, Throwable)` signature is call-compatible.)
|
||||
|
||||
- [ ] **Step 4: Verify no leftover SLF4J references**
|
||||
|
||||
Run: `grep -n "slf4j\|LoggerFactory" plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java`
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 5: Compile the tiles plugin (confirms log4j-api resolves transitively)**
|
||||
|
||||
Run: `mvn -q -o compile -pl plugins/tiles -am -DskipAssembly` (drop `-o` if downloads are needed)
|
||||
Expected: BUILD SUCCESS. If it fails with `package org.apache.logging.log4j does not exist`, STOP — the transitive assumption is wrong; report back before adding any dependency.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java
|
||||
git commit -m "WW-5620 Migrate AbstractDefaultToStringRenderable to Log4j2
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Remove the injectable Logger factory from `ContainerBuilder`
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/src/main/java/org/apache/struts2/inject/ContainerBuilder.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: nothing from other tasks.
|
||||
- Produces: removes the DI binding for `java.util.logging.Logger` (verified unused repo-wide).
|
||||
|
||||
- [ ] **Step 1: Re-confirm the injectable Logger is unused**
|
||||
|
||||
Run: `grep -rn --include="*.java" "java.util.logging.Logger" . | grep -v /target/ | grep -v "ContainerBuilder.java\|FinalizableReferenceQueue.java"`
|
||||
Expected: no matches (after Task 1, `FinalizableReferenceQueue` no longer appears either). If any consumer appears, STOP and report.
|
||||
|
||||
- [ ] **Step 2: Remove the `LOGGER_FACTORY` constant**
|
||||
|
||||
Delete this block:
|
||||
```java
|
||||
private static final InternalFactory<Logger> LOGGER_FACTORY =
|
||||
new InternalFactory<>() {
|
||||
public Logger create(InternalContext context) {
|
||||
Member member = context.getExternalContext().getMember();
|
||||
return member == null ? Logger.getAnonymousLogger()
|
||||
: Logger.getLogger(member.getDeclaringClass().getName());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Class<? extends Logger> type() {
|
||||
return Logger.class;
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Remove the factory registration in the constructor**
|
||||
|
||||
Delete these lines (comment + registration):
|
||||
```java
|
||||
// Inject the logger for the injected member's declaring class.
|
||||
factories.put(Key.newInstance(Logger.class, Container.DEFAULT_NAME), LOGGER_FACTORY);
|
||||
```
|
||||
Leave the preceding `CONTAINER_FACTORY` registration intact.
|
||||
|
||||
- [ ] **Step 4: Remove the now-unused imports**
|
||||
|
||||
Delete:
|
||||
```java
|
||||
import java.lang.reflect.Member;
|
||||
```
|
||||
and
|
||||
```java
|
||||
import java.util.logging.Logger;
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Update the class Javadoc**
|
||||
|
||||
In the class Javadoc `<ul>`, delete the line:
|
||||
```java
|
||||
* <li>Injects the {@link Logger} for the injected member's declaring class.
|
||||
```
|
||||
Leave the `<li>Injects the current {@link Container}.` bullet.
|
||||
|
||||
- [ ] **Step 6: Verify no leftover references**
|
||||
|
||||
Run: `grep -n "Logger\|Member\|LOGGER_FACTORY" core/src/main/java/org/apache/struts2/inject/ContainerBuilder.java`
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 7: Compile core**
|
||||
|
||||
Run: `mvn -q -o compile -pl core -DskipAssembly` (drop `-o` if downloads are needed)
|
||||
Expected: BUILD SUCCESS.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add core/src/main/java/org/apache/struts2/inject/ContainerBuilder.java
|
||||
git commit -m "WW-5620 Remove unused injectable j.u.l.Logger DI factory from ContainerBuilder
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Remove dead first-party SLF4J dependency declarations
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/pom.xml`
|
||||
- Modify: `parent/pom.xml`
|
||||
- Modify: `pom.xml`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: Tasks 1–3 must be done first (no first-party source uses SLF4J anymore).
|
||||
- Produces: nothing consumed by other tasks.
|
||||
|
||||
- [ ] **Step 1: Dependency-tree gate (before deleting anything)**
|
||||
|
||||
Run: `grep -rln --include="*.java" "org.slf4j" . | grep -v /target/`
|
||||
Expected: no matches (Task 2 removed the last usage).
|
||||
|
||||
Run: `mvn -q -o dependency:tree -pl core,plugins/tiles -Dincludes=org.slf4j 2>&1 | grep -i slf4j` (drop `-o` if downloads are needed)
|
||||
Expected: any `org.slf4j:*` shown is transitive (an indented child of another artifact), not a direct first-party declaration. If a first-party module *requires* Struts to expose SLF4J for compilation, STOP, keep the minimum needed, and note it in the spec's Verification section before continuing.
|
||||
|
||||
- [ ] **Step 2: Remove the SLF4J block from `core/pom.xml`**
|
||||
|
||||
Delete:
|
||||
```xml
|
||||
<!-- SLF4J support -->
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
<optional>true</optional>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-simple</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Remove the SLF4J `dependencyManagement` entries from `parent/pom.xml`**
|
||||
|
||||
Delete:
|
||||
```xml
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
<version>${slf4j.version}</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-simple</artifactId>
|
||||
<version>${slf4j.version}</version>
|
||||
</dependency>
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Remove the `slf4j.version` property from root `pom.xml`**
|
||||
|
||||
Delete:
|
||||
```xml
|
||||
<slf4j.version>2.0.18</slf4j.version>
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Verify no SLF4J declarations remain**
|
||||
|
||||
Run: `grep -rn "slf4j" --include="pom.xml" . | grep -v /target/`
|
||||
Expected: only bridge artifacts remain — `apps/showcase` `log4j-slf4j-impl` (and its `<name>slf4j</name>` profile entry). No `org.slf4j:slf4j-api`, no `slf4j-simple`, no `slf4j.version`.
|
||||
|
||||
- [ ] **Step 6: Build and test core + tiles**
|
||||
|
||||
Run: `mvn -o test -DskipAssembly -pl core -am` (drop `-o` if downloads are needed)
|
||||
Expected: BUILD SUCCESS, tests green.
|
||||
|
||||
Run: `mvn -o test -DskipAssembly -pl plugins/tiles -am`
|
||||
Expected: BUILD SUCCESS, tests green.
|
||||
|
||||
If any test fails because it silently relied on `slf4j-simple` as a test logging backend, STOP and report — do not add unrelated logging config to force a pass.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add core/pom.xml parent/pom.xml pom.xml
|
||||
git commit -m "WW-5620 Remove dead first-party SLF4J dependency declarations
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
**Spec coverage:**
|
||||
- Spec §1 FinalizableReferenceQueue → Task 1. ✓
|
||||
- Spec §2 AbstractDefaultToStringRenderable → Task 2. ✓
|
||||
- Spec §3 ContainerBuilder factory removal → Task 3. ✓
|
||||
- Spec §4 dependency cleanup (core/parent/root poms) → Task 4. ✓
|
||||
- Spec Verification (dependency-tree gate, compile+test, grep) → Task 4 Step 1, per-task compile steps, grep steps. ✓
|
||||
- Out-of-scope bridges left untouched → asserted in Global Constraints and Task 4 Step 5. ✓
|
||||
|
||||
**Placeholder scan:** No TBD/TODO/"handle edge cases"/vague steps. Every code step shows exact before/after. ✓
|
||||
|
||||
**Type consistency:** Logger field named `LOG` (static, Task 1) vs `log` (instance, Task 2) — intentional, matches each file's existing style and the codebase convention; no cross-task symbol sharing. ✓
|
||||
@@ -0,0 +1,135 @@
|
||||
# WW-5620 — Standardize logging on Log4j2
|
||||
|
||||
- **Jira:** [WW-5620](https://issues.apache.org/jira/browse/WW-5620)
|
||||
- **Type:** Improvement (refactor / dependency cleanup)
|
||||
- **Fix version:** 7.3.0
|
||||
- **Components:** Core, Plugin - Tiles
|
||||
- **Date:** 2026-07-20
|
||||
|
||||
## Goal
|
||||
|
||||
Log4j2 is the project's standard logging layer. Three source files still use
|
||||
legacy logging frameworks. Migrate them to Log4j2 and remove the now-dead
|
||||
first-party SLF4J dependency declarations that only existed to support them.
|
||||
|
||||
This is a pure logging-standardization refactor: **no functional or security
|
||||
behavior changes.** All three files are vendored (files 1 and 3 from Google
|
||||
Guice, file 2 from Apache Tiles), so there is no upstream-sync concern.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
1. Migrate `FinalizableReferenceQueue.java` (core) off `java.util.logging`.
|
||||
2. Migrate `AbstractDefaultToStringRenderable.java` (tiles) off SLF4J.
|
||||
3. Remove the injectable `java.util.logging.Logger` DI factory from
|
||||
`ContainerBuilder.java` (core).
|
||||
4. Remove the first-party SLF4J dependency declarations that become dead after
|
||||
step 2, gated on a build/dependency-tree verification.
|
||||
|
||||
### Out of scope
|
||||
- The commons-logging → Log4j2 and SLF4J → Log4j2 **runtime bridges**
|
||||
(`log4j-jcl`, `log4j-slf4j-impl`, `commons-logging`). These route
|
||||
*transitive third-party* library logging into Log4j2 and are unrelated to
|
||||
first-party code. They stay.
|
||||
- Any broader logging refactor beyond these three files.
|
||||
|
||||
## File-level changes
|
||||
|
||||
### 1. `core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java`
|
||||
|
||||
Faithful class-own logger swap, adopting the codebase convention (`private
|
||||
static final Logger LOG = LogManager.getLogger(<Class>.class);` — see
|
||||
`ActionSupport`, `ObjectFactory`, `DefaultActionInvocation`).
|
||||
|
||||
- Remove imports `java.util.logging.Level`, `java.util.logging.Logger`.
|
||||
- Add imports `org.apache.logging.log4j.LogManager`,
|
||||
`org.apache.logging.log4j.Logger`.
|
||||
- Replace
|
||||
`private static final Logger logger = Logger.getLogger(FinalizableReferenceQueue.class.getName());`
|
||||
with
|
||||
`private static final Logger LOG = LogManager.getLogger(FinalizableReferenceQueue.class);`
|
||||
- Replace
|
||||
`logger.log(Level.SEVERE, "Error cleaning up after reference.", t);`
|
||||
with
|
||||
`LOG.error("Error cleaning up after reference.", t);`
|
||||
|
||||
### 2. `plugins/tiles/src/main/java/org/apache/tiles/velocity/template/AbstractDefaultToStringRenderable.java`
|
||||
|
||||
Faithful SLF4J swap. The Log4j2 `Logger` error API is call-compatible, so only
|
||||
imports and the factory call change.
|
||||
|
||||
- Remove imports `org.slf4j.Logger`, `org.slf4j.LoggerFactory`.
|
||||
- Add imports `org.apache.logging.log4j.LogManager`,
|
||||
`org.apache.logging.log4j.Logger`.
|
||||
- Replace `private final Logger log = LoggerFactory.getLogger(getClass());`
|
||||
with `private final Logger log = LogManager.getLogger(getClass());`
|
||||
- `log.error("Error when closing a StringWriter, the impossible happened!", e);`
|
||||
is unchanged.
|
||||
- No new pom dependency: `log4j-api` reaches the tiles plugin transitively via
|
||||
`struts2-core`. Confirm at build time.
|
||||
|
||||
### 3. `core/src/main/java/org/apache/struts2/inject/ContainerBuilder.java`
|
||||
|
||||
Here `java.util.logging.Logger` is **not** a class-own logger — it is
|
||||
registered as an injectable DI type (a legacy Guice feature: consumers could
|
||||
`@Inject` a `Logger` scoped to their declaring class). A repo-wide search found
|
||||
**no code that injects it**. Per decision, remove the dead feature rather than
|
||||
port it to Log4j2 (which has no `getAnonymousLogger()` equivalent for the
|
||||
`member == null` fallback and would change the DI contract).
|
||||
|
||||
- Remove the `LOGGER_FACTORY` `InternalFactory<Logger>` constant.
|
||||
- Remove its registration:
|
||||
`factories.put(Key.newInstance(Logger.class, Container.DEFAULT_NAME), LOGGER_FACTORY);`
|
||||
and the preceding comment.
|
||||
- Remove now-unused imports `java.util.logging.Logger` and
|
||||
`java.lang.reflect.Member` (`Member` is used only inside the removed factory).
|
||||
- Update the class Javadoc: drop the
|
||||
"Injects the `{@link Logger}` for the injected member's declaring class"
|
||||
bullet, leaving the `Container` bullet.
|
||||
|
||||
### 4. Dependency cleanup (gated on verification)
|
||||
|
||||
Evidence for removal:
|
||||
- The tiles file (step 2) is the only main-source SLF4J user in the repo.
|
||||
- No test source uses SLF4J; there is no `simplelogger.properties`.
|
||||
- core's `slf4j-api` is `optional` (never part of the transitive API contract)
|
||||
and dates to the original xwork merge.
|
||||
|
||||
Remove, after the verification gate below:
|
||||
- `core/pom.xml` — the optional `slf4j-api` dependency and the test-scoped
|
||||
`slf4j-simple` dependency (the `<!-- SLF4J support -->` block).
|
||||
- `parent/pom.xml` — the `dependencyManagement` entries for `slf4j-api` and
|
||||
`slf4j-simple`.
|
||||
- root `pom.xml` — the `<slf4j.version>` property.
|
||||
|
||||
`java.util.logging` is JDK built-in — nothing to remove for the two core files.
|
||||
|
||||
## Verification
|
||||
|
||||
1. **Dependency-tree gate (do before deleting pom entries):**
|
||||
run `mvn dependency:tree` for `core` and `plugins/tiles`. Confirm no
|
||||
first-party module *requires* Struts to expose SLF4J. If something genuinely
|
||||
needs it, keep that minimum and record the reason here.
|
||||
2. **Compile + test:**
|
||||
- `mvn test -DskipAssembly -pl core` (core tests are JUnit 4 in this repo)
|
||||
- `mvn test -DskipAssembly -pl plugins/tiles`
|
||||
3. **Grep confirmation:** no source references the injectable `Logger` type,
|
||||
the removed `logger` field, or `org.slf4j` after the change.
|
||||
|
||||
## Testing strategy
|
||||
|
||||
No new tests are required:
|
||||
- Files 1 and 2 are behavior-preserving logger swaps.
|
||||
- File 3 removes dead, untested code.
|
||||
|
||||
Existing core and tiles test suites must stay green.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Low.** The only non-mechanical change is removing the injectable `Logger`
|
||||
factory in `ContainerBuilder`, which is unused internally. A third-party
|
||||
extension that injected `java.util.logging.Logger` from the Struts container
|
||||
would lose that binding; this is acceptable for the 7.3.0 minor and called out
|
||||
here.
|
||||
- The dependency-tree gate protects against an unexpected transitive reliance on
|
||||
the SLF4J declarations.
|
||||
@@ -274,17 +274,6 @@
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-api</artifactId>
|
||||
<version>${slf4j.version}</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.slf4j</groupId>
|
||||
<artifactId>slf4j-simple</artifactId>
|
||||
<version>${slf4j.version}</version>
|
||||
</dependency>
|
||||
|
||||
<dependency>
|
||||
<groupId>org.apache.logging.log4j</groupId>
|
||||
<artifactId>log4j-bom</artifactId>
|
||||
|
||||
+3
-3
@@ -26,14 +26,14 @@ import java.util.Map;
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.tiles.velocity.TilesVelocityException;
|
||||
import org.apache.velocity.context.Context;
|
||||
import org.apache.velocity.exception.MethodInvocationException;
|
||||
import org.apache.velocity.exception.ParseErrorException;
|
||||
import org.apache.velocity.exception.ResourceNotFoundException;
|
||||
import org.apache.velocity.runtime.Renderable;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
/**
|
||||
* Renderable that provides a default implementation of Renderable#toString()
|
||||
@@ -74,7 +74,7 @@ public abstract class AbstractDefaultToStringRenderable implements Renderable {
|
||||
/**
|
||||
* The logging object.
|
||||
*/
|
||||
private final Logger log = LoggerFactory.getLogger(getClass());
|
||||
private final Logger log = LogManager.getLogger(getClass());
|
||||
|
||||
/**
|
||||
* Constructor.
|
||||
|
||||
@@ -125,7 +125,6 @@
|
||||
<log4j2.version>2.26.1</log4j2.version>
|
||||
<mockito.version>5.23.0</mockito.version>
|
||||
<ognl.version>3.4.11</ognl.version>
|
||||
<slf4j.version>2.0.18</slf4j.version>
|
||||
<spring.version>6.2.12</spring.version>
|
||||
<struts-annotations.version>2.0</struts-annotations.version>
|
||||
<velocity-tools.version>3.1</velocity-tools.version>
|
||||
|
||||
Reference in New Issue
Block a user