Skip to content

Commit 3a91665

Browse files
ppkarwaszvy
authored and
jvz
committed
Use garbage-free formatter for s and S patterns (#3338)
This PR improves #3139, by introducing a new `InstantPatternFormatter` for patterns of the form "ss\.S{n}". Unlike the previous formatter based on `DateTimeFormatter`, the formatter is garbage-free. We also simplify the merging algorithm for pattern formatter factories, by moving the merging logic to the pattern formatter factories themselves. This PR does not contain a separate change log entry, since #3139 has not been published yet. Fixes #3337. Co-authored-by: Volkan Yazıcı <[email protected]>
1 parent 0aeb58c commit 3a91665

File tree

5 files changed

+459
-442
lines changed

5 files changed

+459
-442
lines changed

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

Lines changed: 101 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@
3232
import java.util.stream.IntStream;
3333
import java.util.stream.Stream;
3434
import org.apache.logging.log4j.core.time.MutableInstant;
35-
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.CompositePatternSequence;
3635
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.DynamicPatternSequence;
3736
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.PatternSequence;
37+
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.SecondPatternSequence;
3838
import org.apache.logging.log4j.core.util.internal.instant.InstantPatternDynamicFormatter.StaticPatternSequence;
3939
import org.apache.logging.log4j.util.Constants;
4040
import org.junit.jupiter.params.ParameterizedTest;
@@ -55,104 +55,74 @@ void sequencing_should_work(
5555
static List<Arguments> sequencingTestCases() {
5656
final List<Arguments> testCases = new ArrayList<>();
5757

58+
// Merged constants
59+
testCases.add(Arguments.of(":'foo',", ChronoUnit.DAYS, singletonList(new StaticPatternSequence(":foo,"))));
60+
5861
// `SSSX` should be treated constant for daily updates
59-
testCases.add(Arguments.of("SSSX", ChronoUnit.DAYS, singletonList(pCom(pDyn("SSS"), pDyn("X")))));
62+
testCases.add(Arguments.of("SSSX", ChronoUnit.DAYS, asList(pMilliSec(), pDyn("X"))));
6063

6164
// `yyyyMMddHHmmssSSSX` instant cache updated hourly
6265
testCases.add(Arguments.of(
6366
"yyyyMMddHHmmssSSSX",
6467
ChronoUnit.HOURS,
65-
asList(
66-
pCom(pDyn("yyyy"), pDyn("MM"), pDyn("dd"), pDyn("HH")),
67-
pCom(pDyn("mm"), pDyn("ss"), pDyn("SSS")),
68-
pDyn("X"))));
68+
asList(pDyn("yyyyMMddHH", ChronoUnit.HOURS), pDyn("mm"), pSec("", 3), pDyn("X"))));
6969

7070
// `yyyyMMddHHmmssSSSX` instant cache updated per minute
7171
testCases.add(Arguments.of(
7272
"yyyyMMddHHmmssSSSX",
7373
ChronoUnit.MINUTES,
74-
asList(
75-
pCom(pDyn("yyyy"), pDyn("MM"), pDyn("dd"), pDyn("HH"), pDyn("mm")),
76-
pCom(pDyn("ss"), pDyn("SSS")),
77-
pDyn("X"))));
74+
asList(pDyn("yyyyMMddHHmm", ChronoUnit.MINUTES), pSec("", 3), pDyn("X"))));
7875

7976
// ISO9601 instant cache updated daily
8077
final String iso8601InstantPattern = "yyyy-MM-dd'T'HH:mm:ss.SSSX";
8178
testCases.add(Arguments.of(
8279
iso8601InstantPattern,
8380
ChronoUnit.DAYS,
8481
asList(
85-
pCom(pDyn("yyyy"), pSta("-"), pDyn("MM"), pSta("-"), pDyn("dd"), pSta("T")),
86-
pCom(
87-
pDyn("HH"),
88-
pSta(":"),
89-
pDyn("mm"),
90-
pSta(":"),
91-
pDyn("ss"),
92-
pSta("."),
93-
pDyn("SSS"),
94-
pDyn("X")))));
82+
pDyn("yyyy'-'MM'-'dd'T'", ChronoUnit.DAYS),
83+
pDyn("HH':'mm':'", ChronoUnit.MINUTES),
84+
pSec(".", 3),
85+
pDyn("X"))));
9586

9687
// ISO9601 instant cache updated per minute
9788
testCases.add(Arguments.of(
9889
iso8601InstantPattern,
9990
ChronoUnit.MINUTES,
100-
asList(
101-
pCom(
102-
pDyn("yyyy"),
103-
pSta("-"),
104-
pDyn("MM"),
105-
pSta("-"),
106-
pDyn("dd"),
107-
pSta("T"),
108-
pDyn("HH"),
109-
pSta(":"),
110-
pDyn("mm"),
111-
pSta(":")),
112-
pCom(pDyn("ss"), pSta("."), pDyn("SSS")),
113-
pDyn("X"))));
91+
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
11492

11593
// ISO9601 instant cache updated per second
11694
testCases.add(Arguments.of(
11795
iso8601InstantPattern,
11896
ChronoUnit.SECONDS,
119-
asList(
120-
pCom(
121-
pDyn("yyyy"),
122-
pSta("-"),
123-
pDyn("MM"),
124-
pSta("-"),
125-
pDyn("dd"),
126-
pSta("T"),
127-
pDyn("HH"),
128-
pSta(":"),
129-
pDyn("mm"),
130-
pSta(":"),
131-
pDyn("ss"),
132-
pSta(".")),
133-
pDyn("SSS"),
134-
pDyn("X"))));
97+
asList(pDyn("yyyy'-'MM'-'dd'T'HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 3), pDyn("X"))));
98+
99+
// Seconds and micros
100+
testCases.add(Arguments.of(
101+
"HH:mm:ss.SSSSSS", ChronoUnit.MINUTES, asList(pDyn("HH':'mm':'", ChronoUnit.MINUTES), pSec(".", 6))));
135102

136103
return testCases;
137104
}
138105

139-
private static CompositePatternSequence pCom(final PatternSequence... sequences) {
140-
return new CompositePatternSequence(asList(sequences));
106+
private static DynamicPatternSequence pDyn(final String singlePattern) {
107+
return new DynamicPatternSequence(singlePattern);
108+
}
109+
110+
private static DynamicPatternSequence pDyn(final String pattern, final ChronoUnit precision) {
111+
return new DynamicPatternSequence(pattern, precision);
141112
}
142113

143-
private static DynamicPatternSequence pDyn(final String pattern) {
144-
return new DynamicPatternSequence(pattern);
114+
private static SecondPatternSequence pSec(String separator, int fractionalDigits) {
115+
return new SecondPatternSequence(true, separator, fractionalDigits);
145116
}
146117

147-
private static StaticPatternSequence pSta(final String literal) {
148-
return new StaticPatternSequence(literal);
118+
private static SecondPatternSequence pMilliSec() {
119+
return new SecondPatternSequence(false, "", 3);
149120
}
150121

151122
@ParameterizedTest
152123
@ValueSource(
153124
strings = {
154125
// Basics
155-
"S",
156126
"SSSSSSS",
157127
"SSSSSSSSS",
158128
"n",
@@ -163,8 +133,7 @@ private static StaticPatternSequence pSta(final String literal) {
163133
"yyyy-MM-dd HH:mm:ss,SSSSSSS",
164134
"yyyy-MM-dd HH:mm:ss,SSSSSSSS",
165135
"yyyy-MM-dd HH:mm:ss,SSSSSSSSS",
166-
"yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS",
167-
"yyyy-MM-dd'T'HH:mm:ss.SXXX"
136+
"yyyy-MM-dd'T'HH:mm:ss.SSSSSSSSS"
168137
})
169138
void should_recognize_patterns_of_nano_precision(final String pattern) {
170139
assertPatternPrecision(pattern, ChronoUnit.NANOS);
@@ -233,20 +202,31 @@ void should_recognize_patterns_of_second_precision(final String pattern) {
233202
assertPatternPrecision(pattern, ChronoUnit.SECONDS);
234203
}
235204

236-
@ParameterizedTest
237-
@ValueSource(
238-
strings = {
205+
static Stream<String> should_recognize_patterns_of_minute_precision() {
206+
Stream<String> stream = Stream.of(
239207
// Basics
240208
"m",
241209
"mm",
210+
"Z",
211+
"x",
212+
"X",
213+
"O",
214+
"z",
215+
"VV",
242216
// Mixed with other stuff
243217
"yyyy-MM-dd HH:mm",
244218
"yyyy-MM-dd'T'HH:mm",
245219
"HH:mm",
220+
"yyyy-MM-dd HH x",
221+
"yyyy-MM-dd'T'HH XX",
246222
// Single-quoted text containing nanosecond and millisecond directives
247223
"yyyy-MM-dd'S'HH:mm",
248-
"yyyy-MM-dd'n'HH:mm"
249-
})
224+
"yyyy-MM-dd'n'HH:mm");
225+
return Constants.JAVA_MAJOR_VERSION > 8 ? Stream.concat(stream, Stream.of("v")) : stream;
226+
}
227+
228+
@ParameterizedTest
229+
@MethodSource
250230
void should_recognize_patterns_of_minute_precision(final String pattern) {
251231
assertPatternPrecision(pattern, ChronoUnit.MINUTES);
252232
}
@@ -267,28 +247,71 @@ static List<String> hourPrecisionPatterns() {
267247
"K",
268248
"k",
269249
"H",
270-
"Z",
271-
"x",
272-
"X",
273-
"O",
274-
"z",
275-
"VV",
276250
// Mixed with other stuff
277251
"yyyy-MM-dd HH",
278252
"yyyy-MM-dd'T'HH",
279-
"yyyy-MM-dd HH x",
280-
"yyyy-MM-dd'T'HH XX",
281253
"ddHH",
282254
// Single-quoted text containing nanosecond and millisecond directives
283255
"yyyy-MM-dd'S'HH",
284256
"yyyy-MM-dd'n'HH"));
285257
if (Constants.JAVA_MAJOR_VERSION > 8) {
286258
java8Patterns.add("B");
287-
java8Patterns.add("v");
288259
}
289260
return java8Patterns;
290261
}
291262

263+
static Stream<Arguments> dynamic_pattern_should_correctly_determine_precision() {
264+
// When no a precise unit is not available, uses the closest smaller unit.
265+
return Stream.of(
266+
Arguments.of("G", ChronoUnit.ERAS),
267+
Arguments.of("u", ChronoUnit.YEARS),
268+
Arguments.of("D", ChronoUnit.DAYS),
269+
Arguments.of("M", ChronoUnit.MONTHS),
270+
Arguments.of("L", ChronoUnit.MONTHS),
271+
Arguments.of("d", ChronoUnit.DAYS),
272+
Arguments.of("Q", ChronoUnit.MONTHS),
273+
Arguments.of("q", ChronoUnit.MONTHS),
274+
Arguments.of("Y", ChronoUnit.YEARS),
275+
Arguments.of("w", ChronoUnit.WEEKS),
276+
Arguments.of("W", ChronoUnit.DAYS), // The month can change in the middle of the week
277+
Arguments.of("F", ChronoUnit.DAYS), // The month can change in the middle of the week
278+
Arguments.of("E", ChronoUnit.DAYS),
279+
Arguments.of("e", ChronoUnit.DAYS),
280+
Arguments.of("c", ChronoUnit.DAYS),
281+
Arguments.of("a", ChronoUnit.HOURS), // Let us round it down
282+
Arguments.of("h", ChronoUnit.HOURS),
283+
Arguments.of("K", ChronoUnit.HOURS),
284+
Arguments.of("k", ChronoUnit.HOURS),
285+
Arguments.of("H", ChronoUnit.HOURS),
286+
Arguments.of("m", ChronoUnit.MINUTES),
287+
Arguments.of("s", ChronoUnit.SECONDS),
288+
Arguments.of("S", ChronoUnit.MILLIS),
289+
Arguments.of("SS", ChronoUnit.MILLIS),
290+
Arguments.of("SSS", ChronoUnit.MILLIS),
291+
Arguments.of("SSSS", ChronoUnit.MICROS),
292+
Arguments.of("SSSSS", ChronoUnit.MICROS),
293+
Arguments.of("SSSSSS", ChronoUnit.MICROS),
294+
Arguments.of("SSSSSSS", ChronoUnit.NANOS),
295+
Arguments.of("SSSSSSSS", ChronoUnit.NANOS),
296+
Arguments.of("SSSSSSSSS", ChronoUnit.NANOS),
297+
Arguments.of("A", ChronoUnit.MILLIS),
298+
Arguments.of("n", ChronoUnit.NANOS),
299+
Arguments.of("N", ChronoUnit.NANOS),
300+
// Time zones can change in the middle of a UTC hour (e.g. India)
301+
Arguments.of("VV", ChronoUnit.MINUTES),
302+
Arguments.of("z", ChronoUnit.MINUTES),
303+
Arguments.of("O", ChronoUnit.MINUTES),
304+
Arguments.of("X", ChronoUnit.MINUTES),
305+
Arguments.of("x", ChronoUnit.MINUTES),
306+
Arguments.of("Z", ChronoUnit.MINUTES));
307+
}
308+
309+
@ParameterizedTest
310+
@MethodSource
311+
void dynamic_pattern_should_correctly_determine_precision(String singlePattern, ChronoUnit expectedPrecision) {
312+
assertThat(pDyn(singlePattern).precision).isEqualTo(expectedPrecision);
313+
}
314+
292315
private static void assertPatternPrecision(final String pattern, final ChronoUnit expectedPrecision) {
293316
final InstantPatternFormatter formatter =
294317
new InstantPatternDynamicFormatter(pattern, Locale.getDefault(), TimeZone.getDefault());
@@ -351,7 +374,11 @@ static Stream<Arguments> formatterInputs(final String pattern) {
351374

352375
private static MutableInstant randomInstant() {
353376
final MutableInstant instant = new MutableInstant();
354-
final long epochSecond = RANDOM.nextInt(1_621_280_470); // 2021-05-17 21:41:10
377+
// In the 1970's some time zones had sub-minute offsets to UTC, e.g., Africa/Monrovia.
378+
// We will exclude them for tests:
379+
final int minEpochSecond = 315_532_800; // 1980-01-01 01:00:00
380+
final int maxEpochSecond = 1_621_280_470; // 2021-05-17 21:41:10
381+
final long epochSecond = minEpochSecond + RANDOM.nextInt(maxEpochSecond - minEpochSecond);
355382
final int epochSecondNano = randomNanos();
356383
instant.initFromEpochSecond(epochSecond, epochSecondNano);
357384
return instant;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ static Object[][] getterTestCases() {
107107
}
108108

109109
@ParameterizedTest
110-
@ValueSource(strings = {"S", "SSSS", "SSSSS", "SSSSSS", "SSSSSSS", "SSSSSSSS", "SSSSSSSSS", "n", "N"})
110+
@ValueSource(strings = {"SSSS", "SSSSS", "SSSSSS", "SSSSSSS", "SSSSSSSS", "SSSSSSSSS", "n", "N"})
111111
void ofMilliPrecision_should_fail_on_inconsistent_precision(final String subMilliPattern) {
112112
final InstantPatternDynamicFormatter dynamicFormatter =
113113
new InstantPatternDynamicFormatter(subMilliPattern, LOCALE, TIME_ZONE);

0 commit comments

Comments
 (0)