From 007b9831ae6c25974f5fb3ff2d7aa78d2aeffbb3 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Sun, 16 Feb 2025 10:25:55 +0100
Subject: [PATCH 1/5] Fix formatting of `s` pattern
The specialized `SecondPatternSequence` formatter is optimized to format the `ss.?S*` patterns, where seconds are padded. For the not padded (and uncommon) `s` pattern, we should use the usual `DynamicPatternSequence`.
---
.../instant/InstantPatternDynamicFormatterTest.java | 7 ++++++-
.../internal/instant/InstantPatternDynamicFormatter.java | 6 +++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
index 7829d234aaa..b8faab32ddf 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
@@ -100,6 +100,9 @@ static List<Arguments> sequencingTestCases() {
testCases.add(Arguments.of(
"HH:mm:ss.SSSSSS", ChronoUnit.MINUTES, asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 6))));
+ // Seconds without padding
+ testCases.add(Arguments.of("s.SSS", ChronoUnit.SECONDS, asList(pDyn("s'.'", ChronoUnit.SECONDS), pMilliSec())));
+
return testCases;
}
@@ -352,7 +355,9 @@ static Stream<Arguments> formatterInputs() {
"yyyy-MM-dd'T'HH:mm:ss.SSS",
"yyyy-MM-dd'T'HH:mm:ss.SSSSSS",
"dd/MM/yy HH:mm:ss.SSS",
- "dd/MM/yyyy HH:mm:ss.SSS")
+ "dd/MM/yyyy HH:mm:ss.SSS",
+ // seconds without padding
+ "s.SSS")
.flatMap(InstantPatternDynamicFormatterTest::formatterInputs);
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
index bb8059329ea..c0e14ecdbeb 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
@@ -239,7 +239,11 @@ private static List<PatternSequence> sequencePattern(final String pattern) {
final PatternSequence sequence;
switch (c) {
case 's':
- sequence = new SecondPatternSequence(true, "", 0);
+ if (sequenceContent.length() == 2) {
+ sequence = new SecondPatternSequence(true, "", 0);
+ } else {
+ sequence = new DynamicPatternSequence(sequenceContent);
+ }
break;
case 'S':
sequence = new SecondPatternSequence(false, "", sequenceContent.length());
From 34597ddf1c87294da6b1e7e6a544fc355de66d9d Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Tue, 18 Feb 2025 22:34:13 +0100
Subject: [PATCH 2/5] Add support for unpadded seconds
---
.../InstantPatternDynamicFormatterTest.java | 24 ++++---
.../InstantPatternDynamicFormatter.java | 69 +++++++++++--------
2 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
index b8faab32ddf..307511f5bd4 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
@@ -65,13 +65,13 @@ static List<Arguments> sequencingTestCases() {
testCases.add(Arguments.of(
"yyyyMMddHHmmssSSSX",
ChronoUnit.HOURS,
- asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec("", 3), pDyn("X"))));
+ asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec(2, "", 3), pDyn("X"))));
// `yyyyMMddHHmmssSSSX` instant cache updated per minute
testCases.add(Arguments.of(
"yyyyMMddHHmmssSSSX",
ChronoUnit.MINUTES,
- asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec("", 3), pDyn("X"))));
+ asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec(2, "", 3), pDyn("X"))));
// ISO9601 instant cache updated daily
final String iso8601InstantPattern = "yyyy-MM-dd'T'HH:mm:ss.SSSX";
@@ -81,27 +81,29 @@ static List<Arguments> sequencingTestCases() {
asList(
pDyn("yyyy'-'MM'-'dd'T'", ChronoUnit.DAYS),
pDyn("HH':'mm':'", ChronoUnit.MINUTES),
- pSec(".", 3),
+ pSec(2, ".", 3),
pDyn("X"))));
// ISO9601 instant cache updated per minute
testCases.add(Arguments.of(
iso8601InstantPattern,
ChronoUnit.MINUTES,
- asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
+ asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 3), pDyn("X"))));
// ISO9601 instant cache updated per second
testCases.add(Arguments.of(
iso8601InstantPattern,
ChronoUnit.SECONDS,
- asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
+ asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 3), pDyn("X"))));
// Seconds and micros
testCases.add(Arguments.of(
- "HH:mm:ss.SSSSSS", ChronoUnit.MINUTES, asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 6))));
+ "HH:mm:ss.SSSSSS",
+ ChronoUnit.MINUTES,
+ asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 6))));
// Seconds without padding
- testCases.add(Arguments.of("s.SSS", ChronoUnit.SECONDS, asList(pDyn("s'.'", ChronoUnit.SECONDS), pMilliSec())));
+ testCases.add(Arguments.of("s.SSS", ChronoUnit.SECONDS, singletonList(pSec(1, ".", 3))));
return testCases;
}
@@ -114,12 +116,12 @@ private static DynamicPatternSequence pDyn(final String pattern, final ChronoUni
return new DynamicPatternSequence(pattern, precision);
}
- private static SecondPatternSequence pSec(String separator, int fractionalDigits) {
- return new SecondPatternSequence(true, separator, fractionalDigits);
+ private static SecondPatternSequence pSec(int secondDigits, String separator, int fractionalDigits) {
+ return new SecondPatternSequence(secondDigits, separator, fractionalDigits);
}
private static SecondPatternSequence pMilliSec() {
- return new SecondPatternSequence(false, "", 3);
+ return new SecondPatternSequence(0, "", 3);
}
@ParameterizedTest
@@ -369,7 +371,7 @@ static Stream<Arguments> formatterInputs() {
Arrays.stream(TimeZone.getAvailableIDs()).map(TimeZone::getTimeZone).toArray(TimeZone[]::new);
static Stream<Arguments> formatterInputs(final String pattern) {
- return IntStream.range(0, 500).mapToObj(ignoredIndex -> {
+ return IntStream.range(0, 100).mapToObj(ignoredIndex -> {
final Locale locale = LOCALES[RANDOM.nextInt(LOCALES.length)];
final TimeZone timeZone = TIME_ZONES[RANDOM.nextInt(TIME_ZONES.length)];
final MutableInstant instant = randomInstant();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
index c0e14ecdbeb..1d1732ef230 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
@@ -239,14 +239,10 @@ private static List<PatternSequence> sequencePattern(final String pattern) {
final PatternSequence sequence;
switch (c) {
case 's':
- if (sequenceContent.length() == 2) {
- sequence = new SecondPatternSequence(true, "", 0);
- } else {
- sequence = new DynamicPatternSequence(sequenceContent);
- }
+ sequence = new SecondPatternSequence(sequenceContent.length(), "", 0);
break;
case 'S':
- sequence = new SecondPatternSequence(false, "", sequenceContent.length());
+ sequence = new SecondPatternSequence(0, "", sequenceContent.length());
break;
default:
sequence = new DynamicPatternSequence(sequenceContent);
@@ -698,39 +694,50 @@ static class SecondPatternSequence extends PatternSequence {
100_000_000, 10_000_000, 1_000_000, 100_000, 10_000, 1_000, 100, 10, 1
};
- private final boolean printSeconds;
+ private final int secondDigits;
private final String separator;
private final int fractionalDigits;
- SecondPatternSequence(boolean printSeconds, String separator, int fractionalDigits) {
+ SecondPatternSequence(int secondDigits, String separator, int fractionalDigits) {
super(
- createPattern(printSeconds, separator, fractionalDigits),
- determinePrecision(printSeconds, fractionalDigits));
- this.printSeconds = printSeconds;
+ createPattern(secondDigits, separator, fractionalDigits),
+ determinePrecision(secondDigits, fractionalDigits));
+ if (secondDigits < 0 || secondDigits > 2) {
+ throw new IllegalArgumentException("Unsupported number of `s` pattern letters.");
+ }
+ if (fractionalDigits > 9) {
+ throw new IllegalArgumentException("Unsupported number of `S` pattern letters.");
+ }
+ this.secondDigits = secondDigits;
this.separator = separator;
this.fractionalDigits = fractionalDigits;
}
- private static String createPattern(boolean printSeconds, String separator, int fractionalDigits) {
- StringBuilder builder = new StringBuilder();
- if (printSeconds) {
- builder.append("ss");
- }
- builder.append(StaticPatternSequence.escapeLiteral(separator));
- if (fractionalDigits > 0) {
- builder.append(Strings.repeat("S", fractionalDigits));
- }
- return builder.toString();
+ private static String createPattern(int secondDigits, String separator, int fractionalDigits) {
+ return Strings.repeat("s", secondDigits)
+ + StaticPatternSequence.escapeLiteral(separator)
+ + Strings.repeat("S", fractionalDigits);
}
- private static ChronoUnit determinePrecision(boolean printSeconds, int digits) {
+ private static ChronoUnit determinePrecision(int secondDigits, int digits) {
if (digits > 6) return ChronoUnit.NANOS;
if (digits > 3) return ChronoUnit.MICROS;
if (digits > 0) return ChronoUnit.MILLIS;
- return printSeconds ? ChronoUnit.SECONDS : ChronoUnit.FOREVER;
+ return secondDigits > 0 ? ChronoUnit.SECONDS : ChronoUnit.FOREVER;
+ }
+
+ private void formatSeconds(StringBuilder buffer, Instant instant) {
+ switch (secondDigits) {
+ case 1:
+ buffer.append(instant.getEpochSecond() % 60L);
+ break;
+ case 2:
+ formatPaddedSeconds(buffer, instant);
+ break;
+ }
}
- private static void formatSeconds(StringBuilder buffer, Instant instant) {
+ private static void formatPaddedSeconds(StringBuilder buffer, Instant instant) {
long secondsInMinute = instant.getEpochSecond() % 60L;
buffer.append((char) ((secondsInMinute / 10L) + '0'));
buffer.append((char) ((secondsInMinute % 10L) + '0'));
@@ -761,9 +768,11 @@ private static void formatMillis(StringBuilder buffer, Instant instant) {
@Override
InstantPatternFormatter createFormatter(Locale locale, TimeZone timeZone) {
+ final BiConsumer<StringBuilder, Instant> secondDigitsFormatter =
+ secondDigits == 2 ? SecondPatternSequence::formatPaddedSeconds : this::formatSeconds;
final BiConsumer<StringBuilder, Instant> fractionDigitsFormatter =
fractionalDigits == 3 ? SecondPatternSequence::formatMillis : this::formatFractionalDigits;
- if (!printSeconds) {
+ if (secondDigits == 0) {
return new AbstractFormatter(pattern, locale, timeZone, precision) {
@Override
public void formatTo(StringBuilder buffer, Instant instant) {
@@ -776,7 +785,7 @@ public void formatTo(StringBuilder buffer, Instant instant) {
return new AbstractFormatter(pattern, locale, timeZone, precision) {
@Override
public void formatTo(StringBuilder buffer, Instant instant) {
- formatSeconds(buffer, instant);
+ secondDigitsFormatter.accept(buffer, instant);
buffer.append(separator);
}
};
@@ -784,7 +793,7 @@ public void formatTo(StringBuilder buffer, Instant instant) {
return new AbstractFormatter(pattern, locale, timeZone, precision) {
@Override
public void formatTo(StringBuilder buffer, Instant instant) {
- formatSeconds(buffer, instant);
+ secondDigitsFormatter.accept(buffer, instant);
buffer.append(separator);
fractionDigitsFormatter.accept(buffer, instant);
}
@@ -799,15 +808,15 @@ PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision) {
StaticPatternSequence staticOther = (StaticPatternSequence) other;
if (fractionalDigits == 0) {
return new SecondPatternSequence(
- printSeconds, this.separator + staticOther.literal, fractionalDigits);
+ this.secondDigits, this.separator + staticOther.literal, fractionalDigits);
}
}
// We can always append more fractional digits
if (other instanceof SecondPatternSequence) {
SecondPatternSequence secondOther = (SecondPatternSequence) other;
- if (!secondOther.printSeconds && secondOther.separator.isEmpty()) {
+ if (secondOther.secondDigits == 0 && secondOther.separator.isEmpty()) {
return new SecondPatternSequence(
- printSeconds, this.separator, this.fractionalDigits + secondOther.fractionalDigits);
+ this.secondDigits, this.separator, this.fractionalDigits + secondOther.fractionalDigits);
}
}
return null;
From 04c56a9a1f9592be4c25a9c386d6f807525f0438 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Wed, 19 Feb 2025 11:09:12 +0100
Subject: [PATCH 3/5] Improve unpadded formatting
---
.../instant/InstantPatternDynamicFormatter.java | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
index 1d1732ef230..8b0b411f144 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
@@ -726,15 +726,8 @@ private static ChronoUnit determinePrecision(int secondDigits, int digits) {
return secondDigits > 0 ? ChronoUnit.SECONDS : ChronoUnit.FOREVER;
}
- private void formatSeconds(StringBuilder buffer, Instant instant) {
- switch (secondDigits) {
- case 1:
- buffer.append(instant.getEpochSecond() % 60L);
- break;
- case 2:
- formatPaddedSeconds(buffer, instant);
- break;
- }
+ private static void formatUnpaddedSeconds(StringBuilder buffer, Instant instant) {
+ buffer.append(instant.getEpochSecond() % 60L);
}
private static void formatPaddedSeconds(StringBuilder buffer, Instant instant) {
@@ -768,8 +761,9 @@ private static void formatMillis(StringBuilder buffer, Instant instant) {
@Override
InstantPatternFormatter createFormatter(Locale locale, TimeZone timeZone) {
- final BiConsumer<StringBuilder, Instant> secondDigitsFormatter =
- secondDigits == 2 ? SecondPatternSequence::formatPaddedSeconds : this::formatSeconds;
+ final BiConsumer<StringBuilder, Instant> secondDigitsFormatter = secondDigits == 2
+ ? SecondPatternSequence::formatPaddedSeconds
+ : SecondPatternSequence::formatUnpaddedSeconds;
final BiConsumer<StringBuilder, Instant> fractionDigitsFormatter =
fractionalDigits == 3 ? SecondPatternSequence::formatMillis : this::formatFractionalDigits;
if (secondDigits == 0) {
From 0870bacc0ce4298f3d7901b5f98e447e0b0fa8bd Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Wed, 19 Feb 2025 14:17:19 +0100
Subject: [PATCH 4/5] Apply review suggestions (1)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
---
.../instant/InstantPatternDynamicFormatter.java | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
index 8b0b411f144..85cca7a6f91 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
@@ -702,11 +702,19 @@ static class SecondPatternSequence extends PatternSequence {
super(
createPattern(secondDigits, separator, fractionalDigits),
determinePrecision(secondDigits, fractionalDigits));
- if (secondDigits < 0 || secondDigits > 2) {
- throw new IllegalArgumentException("Unsupported number of `s` pattern letters.");
+ final int maxSecondDigits = 2;
+ if (secondDigits > maxSecondDigits) {
+ final String message = String.format(
+ "More than %d `s` pattern letters are not supported, found: %d",
+ maxSecondDigits, secondDigits);
+ throw new IllegalArgumentException(message);
}
- if (fractionalDigits > 9) {
- throw new IllegalArgumentException("Unsupported number of `S` pattern letters.");
+ final int maxFractionalDigits = 9;
+ if (fractionalDigits > maxFractionalDigits) {
+ final String message = String.format(
+ "More than %d `S` pattern letters are not supported, found: %d",
+ maxFractionalDigits, fractionalDigits);
+ throw new IllegalArgumentException(message);
}
this.secondDigits = secondDigits;
this.separator = separator;
From 5ee2f120c3c4a8f7b21b511d750a7df7291d9e34 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <piotr.github@karwasz.org>
Date: Wed, 19 Feb 2025 14:29:01 +0100
Subject: [PATCH 5/5] Formatting
---
.../util/internal/instant/InstantPatternDynamicFormatter.java | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
index 85cca7a6f91..1c9dfab5711 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
@@ -705,8 +705,7 @@ static class SecondPatternSequence extends PatternSequence {
final int maxSecondDigits = 2;
if (secondDigits > maxSecondDigits) {
final String message = String.format(
- "More than %d `s` pattern letters are not supported, found: %d",
- maxSecondDigits, secondDigits);
+ "More than %d `s` pattern letters are not supported, found: %d", maxSecondDigits, secondDigits);
throw new IllegalArgumentException(message);
}
final int maxFractionalDigits = 9;