Skip to content

Commit 7461b68

Browse files
kelunikkelunik
authored
Fix thread-safety in AbstractDateAssert (#3874)
97b642a introduced a regression that broke `AbstractDateAssert` thread-safety by replacing synchronization on `DEFAULT_DATE_FORMATS` with synchronization on the method that parses with the default formats. Such a synchronization cannot work properly with multiple instances of `AbstractDateAssert` because `DEFAULT_DATE_FORMATS` is static. This change fixes the regression by replacing the synchronization on the method level with a `ThreadLocal` that guards access to the current `DEFAULT_DATE_FORMATS` content.
1 parent 015f095 commit 7461b68

2 files changed

Lines changed: 48 additions & 45 deletions

File tree

‎assertj-core/src/main/java/org/assertj/core/api/AbstractDateAssert.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,29 @@ public abstract class AbstractDateAssert<SELF extends AbstractDateAssert<SELF>>
7575
private static final String DATE_FORMAT_PATTERN_SHOULD_NOT_BE_NULL = "Given date format pattern should not be null";
7676
private static final String DATE_FORMAT_SHOULD_NOT_BE_NULL = "Given date format should not be null";
7777

78+
private static boolean lenientParsing = Configuration.LENIENT_DATE_PARSING;
79+
7880
/**
7981
* the default DateFormat used to parse any String date representation.
8082
*/
81-
private static List<DateFormat> DEFAULT_DATE_FORMATS = defaultDateFormats();
82-
private static boolean lenientParsing = Configuration.LENIENT_DATE_PARSING;
83+
private static final ThreadLocal<List<DateFormat>> DEFAULT_DATE_FORMATS = ThreadLocal.withInitial(() -> list(newIsoDateTimeWithMsAndIsoTimeZoneFormat(lenientParsing),
84+
newIsoDateTimeWithMsFormat(lenientParsing),
85+
newTimestampDateFormat(lenientParsing),
86+
newIsoDateTimeWithIsoTimeZoneFormat(lenientParsing),
87+
newIsoDateTimeFormat(lenientParsing),
88+
newIsoDateFormat(lenientParsing)));
8389

8490
@VisibleForTesting
8591
static List<DateFormat> defaultDateFormats() {
86-
if (DEFAULT_DATE_FORMATS == null || defaultDateFormatMustBeRecreated()) {
87-
DEFAULT_DATE_FORMATS = list(newIsoDateTimeWithMsAndIsoTimeZoneFormat(lenientParsing),
88-
newIsoDateTimeWithMsFormat(lenientParsing),
89-
newTimestampDateFormat(lenientParsing),
90-
newIsoDateTimeWithIsoTimeZoneFormat(lenientParsing),
91-
newIsoDateTimeFormat(lenientParsing),
92-
newIsoDateFormat(lenientParsing));
92+
if (defaultDateFormatMustBeRecreated()) {
93+
DEFAULT_DATE_FORMATS.remove();
9394
}
94-
return DEFAULT_DATE_FORMATS;
95+
return DEFAULT_DATE_FORMATS.get();
9596
}
9697

9798
private static boolean defaultDateFormatMustBeRecreated() {
9899
// check default timezone or lenient flag changes, only check one date format since all are configured the same way
99-
DateFormat dateFormat = DEFAULT_DATE_FORMATS.get(0);
100+
DateFormat dateFormat = DEFAULT_DATE_FORMATS.get().get(0);
100101
return !dateFormat.getTimeZone().getID().equals(TimeZone.getDefault().getID()) || dateFormat.isLenient() != lenientParsing;
101102
}
102103

@@ -3600,7 +3601,7 @@ Date parse(String dateAsString) {
36003601
info.representation().toStringOf(dateFormatsInOrderOfUsage())));
36013602
}
36023603

3603-
private synchronized Date parseDateWithDefaultDateFormats(final String dateAsString) {
3604+
private Date parseDateWithDefaultDateFormats(final String dateAsString) {
36043605
return parseDateWith(dateAsString, defaultDateFormats());
36053606
}
36063607

‎assertj-core/src/test/java/org/assertj/core/api/date/DateAssert_isEqualTo_Test.java‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
*/
1313
package org.assertj.core.api.date;
1414

15-
import static java.time.Instant.parse;
15+
import static java.time.ZoneId.systemDefault;
16+
import static java.util.stream.IntStream.range;
1617
import static org.assertj.core.api.Assertions.assertThat;
1718
import static org.assertj.core.api.BDDAssertions.then;
1819
import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
@@ -21,9 +22,11 @@
2122

2223
import java.sql.Timestamp;
2324
import java.time.Instant;
25+
import java.time.LocalDate;
2426
import java.util.Date;
25-
27+
import java.util.stream.Stream;
2628
import org.junit.jupiter.api.Nested;
29+
import org.junit.jupiter.api.Test;
2730
import org.junit.jupiter.api.TestInstance;
2831
import org.junit.jupiter.params.ParameterizedTest;
2932
import org.junit.jupiter.params.provider.Arguments;
@@ -32,6 +35,7 @@
3235

3336
/**
3437
* @author Joel Costigliola
38+
* @author Niklas Keller
3539
*/
3640
class DateAssert_isEqualTo_Test {
3741

@@ -46,13 +50,11 @@ void should_pass(Date actual, Object expected) {
4650
assertThat(actual).isEqualTo(expected);
4751
}
4852

49-
Arguments[] should_pass() {
50-
return new Arguments[] {
51-
arguments(Date.from(parse("1970-01-01T00:00:00.000000001Z")),
52-
Date.from(parse("1970-01-01T00:00:00.000000001Z"))),
53-
arguments(Date.from(parse("1970-01-01T00:00:00.000000001Z")),
54-
Timestamp.from(parse("1970-01-01T00:00:00.000000001Z")))
55-
};
53+
Stream<Arguments> should_pass() {
54+
return Stream.of(arguments(Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
55+
Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z"))),
56+
arguments(Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
57+
Timestamp.from(Instant.parse("1970-01-01T00:00:00.000000001Z"))));
5658
}
5759

5860
@ParameterizedTest
@@ -64,11 +66,9 @@ void should_fail(Date actual, Object expected) {
6466
then(error).isInstanceOf(AssertionFailedError.class);
6567
}
6668

67-
Arguments[] should_fail() {
68-
return new Arguments[] {
69-
arguments(Timestamp.from(parse("1970-01-01T00:00:00.000000001Z")),
70-
Date.from(parse("1970-01-01T00:00:00.000000001Z")))
71-
};
69+
Stream<Arguments> should_fail() {
70+
return Stream.of(arguments(Timestamp.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
71+
Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z"))));
7272
}
7373

7474
}
@@ -84,13 +84,11 @@ void should_pass(Date actual, Instant expected) {
8484
assertThat(actual).isEqualTo(expected);
8585
}
8686

87-
Arguments[] should_pass() {
88-
return new Arguments[] {
89-
arguments(Date.from(parse("1970-01-01T00:00:00.000000001Z")),
90-
parse("1970-01-01T00:00:00.000000001Z")),
91-
arguments(Timestamp.from(parse("1970-01-01T00:00:00.000000001Z")),
92-
parse("1970-01-01T00:00:00.000000001Z"))
93-
};
87+
Stream<Arguments> should_pass() {
88+
return Stream.of(arguments(Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
89+
Instant.parse("1970-01-01T00:00:00.000000001Z")),
90+
arguments(Timestamp.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
91+
Instant.parse("1970-01-01T00:00:00.000000001Z")));
9492
}
9593

9694
}
@@ -106,11 +104,8 @@ void should_pass(Date actual, String expected) {
106104
assertThat(actual).isEqualTo(expected);
107105
}
108106

109-
Arguments[] should_pass() {
110-
return new Arguments[] {
111-
arguments(Date.from(parse("1970-01-01T00:00:00.000000001Z")),
112-
"1970-01-01T00:00:00.000Z")
113-
};
107+
Stream<Arguments> should_pass() {
108+
return Stream.of(arguments(Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z")), "1970-01-01T00:00:00.000Z"));
114109
}
115110

116111
@ParameterizedTest
@@ -122,13 +117,20 @@ void should_fail(Date actual, String expected) {
122117
then(error).isInstanceOf(AssertionFailedError.class);
123118
}
124119

125-
Arguments[] should_fail() {
126-
return new Arguments[] {
127-
arguments(Date.from(parse("1970-01-01T00:00:00.000000001Z")),
128-
"1970-01-01T00:00:00.000000001Z"),
129-
arguments(Timestamp.from(parse("1970-01-01T00:00:00.000000001Z")),
130-
"1970-01-01T00:00:00.000000001Z")
131-
};
120+
Stream<Arguments> should_fail() {
121+
return Stream.of(arguments(Date.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
122+
"1970-01-01T00:00:00.000000001Z"),
123+
arguments(Timestamp.from(Instant.parse("1970-01-01T00:00:00.000000001Z")),
124+
"1970-01-01T00:00:00.000000001Z"));
125+
}
126+
127+
// https://github.com/assertj/assertj/issues/3873
128+
@Test
129+
void should_pass_concurrently() {
130+
// GIVEN
131+
Date actual = Date.from(LocalDate.parse("1970-01-01").atStartOfDay(systemDefault()).toInstant());
132+
// WHEN/THEN
133+
range(0, 1000).parallel().forEach(i -> assertThat(actual).isEqualTo("1970-01-01"));
132134
}
133135

134136
}

0 commit comments

Comments
 (0)