Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatting of s pattern #3469

Merged
merged 6 commits into from
Feb 19, 2025
Merged

Fix formatting of s pattern #3469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
007b983
Fix formatting of `s` pattern
ppkarwasz Feb 16, 2025
34597dd
Add support for unpadded seconds
ppkarwasz Feb 18, 2025
48478b1
Merge remote-tracking branch 'apache/2.x' into fix/2.x/single_s_pattern
ppkarwasz Feb 18, 2025
04c56a9
Improve unpadded formatting
ppkarwasz Feb 19, 2025
0870bac
Apply review suggestions (1)
ppkarwasz Feb 19, 2025
5ee2f12
Formatting
ppkarwasz Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions ...g/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -81,24 +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, singletonList(pSec(1, ".", 3))));

return testCases;
}
Expand All @@ -111,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
Expand Down Expand Up @@ -352,7 +357,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);
}

Expand All @@ -364,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();
Expand Down
66 changes: 40 additions & 26 deletions ...a/org/apache/logging/log4j/core/util/internal/instant/InstantPatternDynamicFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,10 @@ private static List<PatternSequence> sequencePattern(final String pattern) {
final PatternSequence sequence;
switch (c) {
case 's':
sequence = new SecondPatternSequence(true, "", 0);
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);
Expand Down Expand Up @@ -694,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));
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);
}
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;
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 static void formatUnpaddedSeconds(StringBuilder buffer, Instant instant) {
buffer.append(instant.getEpochSecond() % 60L);
}

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'));
Expand Down Expand Up @@ -757,9 +768,12 @@ private static void formatMillis(StringBuilder buffer, Instant instant) {

@Override
InstantPatternFormatter createFormatter(Locale locale, TimeZone timeZone) {
final BiConsumer<StringBuilder, Instant> secondDigitsFormatter = secondDigits == 2
? SecondPatternSequence::formatPaddedSeconds
: SecondPatternSequence::formatUnpaddedSeconds;
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) {
Expand All @@ -772,15 +786,15 @@ 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);
}
};
}
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);
}
Expand All @@ -795,15 +809,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;
Expand Down