Skip to content

Commit fe50bb5

Browse files
lukman48ceki
lukman48
authored and
ceki
committed
fix: do not warn when both converterClass and class attributes are specified (Issue #1031)
When both 'converterClass' (deprecated) and 'class' (replacement) attributes are specified in a <conversionRule> configuration element, the behavior should be: 1. Silently use the 'class' attribute value (no deprecation warning needed) 2. Log info messages instead of warnings about the duplicate attributes 3. Maintain backward compatibility - allows older config files with only 'converterClass' to work on newer versions without unnecessary warnings This fixes the issue where legitimate configuration that provides both attributes for compatibility across versions would generate spurious deprecation warnings. Changes: - Modified ConversionRuleAction.validPreconditions() to NOT warn about 'converterClass' when 'class' is also present - Changed duplicate attribute warning to info-level logging instead - Added test case testConversionRuleWithBothAttributes to verify behavior - Added test XML configuration file for the new test case Fixes #1031 Signed-off-by: lukman48 <lukman_uki@yahoo.co.id>
1 parent f8fc692 commit fe50bb5

3 files changed

Lines changed: 41 additions & 4 deletions

File tree

‎logback-classic/src/test/input/joran/conversionRule/conversionRuleBothAttributes.xml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<configuration>
2+
3+
<!-- Test that when both converterClass and class attributes are specified,
4+
class attribute takes precedence and no deprecation warning is issued -->
5+
<conversionRule conversionWord="sample"
6+
converterClass="ch.qos.logback.classic.pattern.SampleConverter"
7+
class="ch.qos.logback.classic.pattern.SampleConverter" />
8+
9+
<appender name="LIST" class="ch.qos.logback.core.testUtil.StringListAppender">
10+
<layout class="ch.qos.logback.classic.PatternLayout">
11+
<Pattern>%sample - %msg</Pattern>
12+
</layout>
13+
</appender>
14+
15+
<root level="debug">
16+
<appender-ref ref="LIST" />
17+
</root>
18+
</configuration>

‎logback-classic/src/test/java/ch/qos/logback/classic/PatternLayoutTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,21 @@ public void testConversionRuleInIncluded() throws JoranException {
284284
assertEquals(SampleConverter.SAMPLE_STR + " - " + msg, sla.strList.get(0));
285285
}
286286

287+
@Test
288+
public void testConversionRuleWithBothAttributes() throws JoranException {
289+
// Test that both converterClass and class attributes can be specified together
290+
// The 'class' attribute should take precedence without deprecation warning
291+
configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "conversionRule/conversionRuleBothAttributes.xml");
292+
root.getAppender("LIST");
293+
String msg = "testConversionRuleWithBothAttributes";
294+
logger.debug(msg);
295+
StringListAppender<ILoggingEvent> sla = (StringListAppender<ILoggingEvent>) root.getAppender("LIST");
296+
assertNotNull(sla);
297+
assertEquals(1, sla.strList.size());
298+
// Verify that the conversion rule works (uses 'class' attribute)
299+
assertEquals(SampleConverter.SAMPLE_STR + " - " + msg, sla.strList.get(0));
300+
}
301+
287302
@Test
288303
public void smokeReplace() {
289304
pl.setPattern("%replace(a1234b){'\\d{4}', 'XXXX'}");

‎logback-core/src/main/java/ch/qos/logback/core/joran/action/ConversionRuleAction.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,24 @@ protected boolean validPreconditions(SaxEventInterpretationContext seic, String
3838
boolean invalidConverterClassAttribute = pv.isInvalidAttribute(CONVERTER_CLASS_ATTRIBUTE);
3939
boolean invalidClassAttribute = pv.isInvalidAttribute(CLASS_ATTRIBUTE);
4040

41-
if(!invalidConverterClassAttribute) {
41+
boolean multipleClassAttributes = (!invalidClassAttribute) && (!invalidConverterClassAttribute);
42+
43+
// If both attributes are specified, silently use 'class' (for backward compatibility)
44+
// Do not warn about deprecation when 'class' is present
45+
if(!invalidConverterClassAttribute && !multipleClassAttributes) {
4246
pv.addWarn("["+CONVERTER_CLASS_ATTRIBUTE +"] attribute is deprecated and replaced by ["+CLASS_ATTRIBUTE+
4347
"]. "+pv.getLocationSuffix());
4448
}
49+
4550
boolean missingClass = invalidClassAttribute && invalidConverterClassAttribute;
4651
if(missingClass) {
4752
pv.addMissingAttributeError(CLASS_ATTRIBUTE);
4853
return false;
4954
}
5055

51-
boolean multipleClassAttributes = (!invalidClassAttribute) && (!invalidConverterClassAttribute);
5256
if(multipleClassAttributes) {
53-
pv.addWarn("Both ["+CONVERTER_CLASS_ATTRIBUTE+"] attribute and ["+CLASS_ATTRIBUTE+"] attribute specified. ");
54-
pv.addWarn( "["+CLASS_ATTRIBUTE+"] attribute will override. ");
57+
pv.addInfo("Both ["+CONVERTER_CLASS_ATTRIBUTE+"] attribute and ["+CLASS_ATTRIBUTE+"] attribute specified. ");
58+
pv.addInfo( "["+CLASS_ATTRIBUTE+"] attribute will be used. ");
5559
}
5660
pv.validateGivenAttribute(CONVERSION_WORD_ATTRIBUTE);
5761
return pv.isValid();

0 commit comments

Comments
 (0)