Skip to content

Commit 0891d6b

Browse files
ppkarwaszvy
ppkarwasz
and
vy
authored
Fix formatting of s pattern (#3469)
Fixes the formatting of the single `s` pattern. Co-authored-by: Volkan Yazıcı <[email protected]>
1 parent ae77c09 commit 0891d6b

File tree

2 files changed

+58
-37
lines changed

2 files changed

+58
-37
lines changed

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java

+18-11
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,13 @@ static List<Arguments> sequencingTestCases() {
6565
testCases.add(Arguments.of(
6666
"yyyyMMddHHmmssSSSX",
6767
ChronoUnit.HOURS,
68-
asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec("", 3), pDyn("X"))));
68+
asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec(2, "", 3), pDyn("X"))));
6969

7070
// `yyyyMMddHHmmssSSSX` instant cache updated per minute
7171
testCases.add(Arguments.of(
7272
"yyyyMMddHHmmssSSSX",
7373
ChronoUnit.MINUTES,
74-
asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec("", 3), pDyn("X"))));
74+
asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec(2, "", 3), pDyn("X"))));
7575

7676
// ISO9601 instant cache updated daily
7777
final String iso8601InstantPattern = "yyyy-MM-dd'T'HH:mm:ss.SSSX";
@@ -81,24 +81,29 @@ static List<Arguments> sequencingTestCases() {
8181
asList(
8282
pDyn("yyyy'-'MM'-'dd'T'", ChronoUnit.DAYS),
8383
pDyn("HH':'mm':'", ChronoUnit.MINUTES),
84-
pSec(".", 3),
84+
pSec(2, ".", 3),
8585
pDyn("X"))));
8686

8787
// ISO9601 instant cache updated per minute
8888
testCases.add(Arguments.of(
8989
iso8601InstantPattern,
9090
ChronoUnit.MINUTES,
91-
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
91+
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 3), pDyn("X"))));
9292

9393
// ISO9601 instant cache updated per second
9494
testCases.add(Arguments.of(
9595
iso8601InstantPattern,
9696
ChronoUnit.SECONDS,
97-
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
97+
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 3), pDyn("X"))));
9898

9999
// Seconds and micros
100100
testCases.add(Arguments.of(
101-
"HH:mm:ss.SSSSSS", ChronoUnit.MINUTES, asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 6))));
101+
"HH:mm:ss.SSSSSS",
102+
ChronoUnit.MINUTES,
103+
asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(2, ".", 6))));
104+
105+
// Seconds without padding
106+
testCases.add(Arguments.of("s.SSS", ChronoUnit.SECONDS, singletonList(pSec(1, ".", 3))));
102107

103108
return testCases;
104109
}
@@ -111,12 +116,12 @@ private static DynamicPatternSequence pDyn(final String pattern, final ChronoUni
111116
return new DynamicPatternSequence(pattern, precision);
112117
}
113118

114-
private static SecondPatternSequence pSec(String separator, int fractionalDigits) {
115-
return new SecondPatternSequence(true, separator, fractionalDigits);
119+
private static SecondPatternSequence pSec(int secondDigits, String separator, int fractionalDigits) {
120+
return new SecondPatternSequence(secondDigits, separator, fractionalDigits);
116121
}
117122

118123
private static SecondPatternSequence pMilliSec() {
119-
return new SecondPatternSequence(false, "", 3);
124+
return new SecondPatternSequence(0, "", 3);
120125
}
121126

122127
@ParameterizedTest
@@ -352,7 +357,9 @@ static Stream<Arguments> formatterInputs() {
352357
"yyyy-MM-dd'T'HH:mm:ss.SSS",
353358
"yyyy-MM-dd'T'HH:mm:ss.SSSSSS",
354359
"dd/MM/yy HH:mm:ss.SSS",
355-
"dd/MM/yyyy HH:mm:ss.SSS")
360+
"dd/MM/yyyy HH:mm:ss.SSS",
361+
// seconds without padding
362+
"s.SSS")
356363
.flatMap(InstantPatternDynamicFormatterTest::formatterInputs);
357364
}
358365

@@ -364,7 +371,7 @@ static Stream<Arguments> formatterInputs() {
364371
Arrays.stream(TimeZone.getAvailableIDs()).map(TimeZone::getTimeZone).toArray(TimeZone[]::new);
365372

366373
static Stream<Arguments> formatterInputs(final String pattern) {
367-
return IntStream.range(0, 500).mapToObj(ignoredIndex -> {
374+
return IntStream.range(0, 100).mapToObj(ignoredIndex -> {
368375
final Locale locale = LOCALES[RANDOM.nextInt(LOCALES.length)];
369376
final TimeZone timeZone = TIME_ZONES[RANDOM.nextInt(TIME_ZONES.length)];
370377
final MutableInstant instant = randomInstant();

‎log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java

+40-26
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,10 @@ private static List<PatternSequence> sequencePattern(final String pattern) {
239239
final PatternSequence sequence;
240240
switch (c) {
241241
case 's':
242-
sequence = new SecondPatternSequence(true, "", 0);
242+
sequence = new SecondPatternSequence(sequenceContent.length(), "", 0);
243243
break;
244244
case 'S':
245-
sequence = new SecondPatternSequence(false, "", sequenceContent.length());
245+
sequence = new SecondPatternSequence(0, "", sequenceContent.length());
246246
break;
247247
default:
248248
sequence = new DynamicPatternSequence(sequenceContent);
@@ -694,39 +694,50 @@ static class SecondPatternSequence extends PatternSequence {
694694
100_000_000, 10_000_000, 1_000_000, 100_000, 10_000, 1_000, 100, 10, 1
695695
};
696696

697-
private final boolean printSeconds;
697+
private final int secondDigits;
698698
private final String separator;
699699
private final int fractionalDigits;
700700

701-
SecondPatternSequence(boolean printSeconds, String separator, int fractionalDigits) {
701+
SecondPatternSequence(int secondDigits, String separator, int fractionalDigits) {
702702
super(
703-
createPattern(printSeconds, separator, fractionalDigits),
704-
determinePrecision(printSeconds, fractionalDigits));
705-
this.printSeconds = printSeconds;
703+
createPattern(secondDigits, separator, fractionalDigits),
704+
determinePrecision(secondDigits, fractionalDigits));
705+
final int maxSecondDigits = 2;
706+
if (secondDigits > maxSecondDigits) {
707+
final String message = String.format(
708+
"More than %d `s` pattern letters are not supported, found: %d", maxSecondDigits, secondDigits);
709+
throw new IllegalArgumentException(message);
710+
}
711+
final int maxFractionalDigits = 9;
712+
if (fractionalDigits > maxFractionalDigits) {
713+
final String message = String.format(
714+
"More than %d `S` pattern letters are not supported, found: %d",
715+
maxFractionalDigits, fractionalDigits);
716+
throw new IllegalArgumentException(message);
717+
}
718+
this.secondDigits = secondDigits;
706719
this.separator = separator;
707720
this.fractionalDigits = fractionalDigits;
708721
}
709722

710-
private static String createPattern(boolean printSeconds, String separator, int fractionalDigits) {
711-
StringBuilder builder = new StringBuilder();
712-
if (printSeconds) {
713-
builder.append("ss");
714-
}
715-
builder.append(StaticPatternSequence.escapeLiteral(separator));
716-
if (fractionalDigits > 0) {
717-
builder.append(Strings.repeat("S", fractionalDigits));
718-
}
719-
return builder.toString();
723+
private static String createPattern(int secondDigits, String separator, int fractionalDigits) {
724+
return Strings.repeat("s", secondDigits)
725+
+ StaticPatternSequence.escapeLiteral(separator)
726+
+ Strings.repeat("S", fractionalDigits);
720727
}
721728

722-
private static ChronoUnit determinePrecision(boolean printSeconds, int digits) {
729+
private static ChronoUnit determinePrecision(int secondDigits, int digits) {
723730
if (digits > 6) return ChronoUnit.NANOS;
724731
if (digits > 3) return ChronoUnit.MICROS;
725732
if (digits > 0) return ChronoUnit.MILLIS;
726-
return printSeconds ? ChronoUnit.SECONDS : ChronoUnit.FOREVER;
733+
return secondDigits > 0 ? ChronoUnit.SECONDS : ChronoUnit.FOREVER;
734+
}
735+
736+
private static void formatUnpaddedSeconds(StringBuilder buffer, Instant instant) {
737+
buffer.append(instant.getEpochSecond() % 60L);
727738
}
728739

729-
private static void formatSeconds(StringBuilder buffer, Instant instant) {
740+
private static void formatPaddedSeconds(StringBuilder buffer, Instant instant) {
730741
long secondsInMinute = instant.getEpochSecond() % 60L;
731742
buffer.append((char) ((secondsInMinute / 10L) + '0'));
732743
buffer.append((char) ((secondsInMinute % 10L) + '0'));
@@ -757,9 +768,12 @@ private static void formatMillis(StringBuilder buffer, Instant instant) {
757768

758769
@Override
759770
InstantPatternFormatter createFormatter(Locale locale, TimeZone timeZone) {
771+
final BiConsumer<StringBuilder, Instant> secondDigitsFormatter = secondDigits == 2
772+
? SecondPatternSequence::formatPaddedSeconds
773+
: SecondPatternSequence::formatUnpaddedSeconds;
760774
final BiConsumer<StringBuilder, Instant> fractionDigitsFormatter =
761775
fractionalDigits == 3 ? SecondPatternSequence::formatMillis : this::formatFractionalDigits;
762-
if (!printSeconds) {
776+
if (secondDigits == 0) {
763777
return new AbstractFormatter(pattern, locale, timeZone, precision) {
764778
@Override
765779
public void formatTo(StringBuilder buffer, Instant instant) {
@@ -772,15 +786,15 @@ public void formatTo(StringBuilder buffer, Instant instant) {
772786
return new AbstractFormatter(pattern, locale, timeZone, precision) {
773787
@Override
774788
public void formatTo(StringBuilder buffer, Instant instant) {
775-
formatSeconds(buffer, instant);
789+
secondDigitsFormatter.accept(buffer, instant);
776790
buffer.append(separator);
777791
}
778792
};
779793
}
780794
return new AbstractFormatter(pattern, locale, timeZone, precision) {
781795
@Override
782796
public void formatTo(StringBuilder buffer, Instant instant) {
783-
formatSeconds(buffer, instant);
797+
secondDigitsFormatter.accept(buffer, instant);
784798
buffer.append(separator);
785799
fractionDigitsFormatter.accept(buffer, instant);
786800
}
@@ -795,15 +809,15 @@ PatternSequence tryMerge(PatternSequence other, ChronoUnit thresholdPrecision) {
795809
StaticPatternSequence staticOther = (StaticPatternSequence) other;
796810
if (fractionalDigits == 0) {
797811
return new SecondPatternSequence(
798-
printSeconds, this.separator + staticOther.literal, fractionalDigits);
812+
this.secondDigits, this.separator + staticOther.literal, fractionalDigits);
799813
}
800814
}
801815
// We can always append more fractional digits
802816
if (other instanceof SecondPatternSequence) {
803817
SecondPatternSequence secondOther = (SecondPatternSequence) other;
804-
if (!secondOther.printSeconds && secondOther.separator.isEmpty()) {
818+
if (secondOther.secondDigits == 0 && secondOther.separator.isEmpty()) {
805819
return new SecondPatternSequence(
806-
printSeconds, this.separator, this.fractionalDigits + secondOther.fractionalDigits);
820+
this.secondDigits, this.separator, this.fractionalDigits + secondOther.fractionalDigits);
807821
}
808822
}
809823
return null;

0 commit comments

Comments
 (0)