Skip to content

Commit 1b64cf4

Browse files
authored
Merge pull request #792 from mivek/refactor/pattern-performance
refactor: improve performance and thread safety
2 parents 8624f13 + 9520638 commit 1b64cf4

9 files changed

Lines changed: 112 additions & 30 deletions

File tree

metarParser-commons/src/main/java/io/github/mivek/internationalization/Messages.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import java.util.ResourceBundle;
66

77
/**
8-
* Messages class for internationalization.
8+
* Messages class for internationalization. Thread-safe via ThreadLocal.
99
*
1010
* @author mivek
1111
*/
@@ -14,15 +14,14 @@ public final class Messages {
1414
private static final Messages INSTANCE = new Messages();
1515
/** Name of the bundle. */
1616
private static final String BUNDLE_NAME = "internationalization.messages";
17-
/** Bundle variable. */
18-
private ResourceBundle fResourceBundle;
17+
/** Per-thread bundle holder — thread-safe, no global Locale.setDefault(). */
18+
private final ThreadLocal<ResourceBundle> bundleHolder =
19+
ThreadLocal.withInitial(() -> ResourceBundle.getBundle(BUNDLE_NAME));
1920

2021
/**
2122
* Private constructor.
2223
*/
23-
private Messages() {
24-
fResourceBundle = ResourceBundle.getBundle(BUNDLE_NAME);
25-
}
24+
private Messages() {}
2625

2726
/**
2827
* @return the Messages instance.
@@ -32,22 +31,30 @@ public static Messages getInstance() {
3231
}
3332

3433
/**
35-
* Sets the locale of the bundle.
34+
* Sets the locale of the bundle for the current thread.
3635
*
3736
* @param locale the locale to set.
3837
*/
3938
public void setLocale(final Locale locale) {
40-
Locale.setDefault(locale);
41-
ResourceBundle.clearCache();
42-
fResourceBundle = ResourceBundle.getBundle(BUNDLE_NAME, locale);
39+
bundleHolder.set(ResourceBundle.getBundle(BUNDLE_NAME, locale));
40+
}
41+
42+
/**
43+
* Clears the locale for the current thread, resetting it to the JVM default.
44+
*
45+
* <p>Must be called in thread-pool environments (e.g., servlets, Spring)
46+
* after each request to prevent locale leakage between tasks on the same thread.
47+
*/
48+
public void clearLocale() {
49+
bundleHolder.remove();
4350
}
4451

4552
/**
4653
* @param message the string to get
4754
* @return the translation of message
4855
*/
4956
public String getString(final String message) {
50-
return fResourceBundle.getString(message);
57+
return bundleHolder.get().getString(message);
5158
}
5259

5360
/**

metarParser-commons/src/main/java/io/github/mivek/utils/Converter.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ public final class Converter {
2424
**/
2525
private static final Double SM_TO_KM = 1.609344;
2626

27+
/** Pattern to parse a visibility string composed of a numeric value and a unit. */
28+
private static final Pattern VISIBILITY_PATTERN = Pattern.compile("(\\d+)([a-z,A-Z]+)");
29+
2730
/**
2831
* Private constructor.
2932
*/
@@ -125,7 +128,7 @@ public static float convertTemperature(final String sign, final String temperatu
125128
* @return The visibility in km as a double
126129
*/
127130
public static Double convertVisibilityToKM(final String visibility) {
128-
final Matcher matcher = Pattern.compile("(\\d+)([a-z,A-Z]+)").matcher(visibility.replace(">", ""));
131+
final Matcher matcher = VISIBILITY_PATTERN.matcher(visibility.replace(">", ""));
129132
if (!matcher.find()) {
130133
return null;
131134
}

metarParser-commons/src/main/resources/internationalization/messages_fr.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ Converter.NNE=Nord Nord Est
202202
Converter.NNW=Nord Nord Ouest
203203
Converter.NSC=Aucun changement significatif
204204
Converter.NW=Nord Ouest
205-
Converter.S=Est
205+
Converter.S=Sud
206206
Converter.SE=Sud Est
207207
Converter.SSE=Sud Sud Est
208208
Converter.SSW=Sud Sud Ouest

metarParser-commons/src/test/java/io/github/mivek/internationalization/MessagesTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
package io.github.mivek.internationalization;
22

3+
import org.junit.jupiter.api.Disabled;
34
import org.junit.jupiter.api.Test;
5+
import org.junit.jupiter.params.ParameterizedTest;
6+
import org.junit.jupiter.params.provider.ValueSource;
47

8+
import java.io.IOException;
9+
import java.io.InputStream;
10+
import java.io.InputStreamReader;
11+
import java.nio.charset.StandardCharsets;
512
import java.util.Locale;
13+
import java.util.Properties;
14+
import java.util.Set;
615

716
import static org.junit.jupiter.api.Assertions.assertEquals;
17+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
18+
import static org.junit.jupiter.api.Assertions.assertTrue;
819

920
class MessagesTest {
1021

@@ -19,4 +30,36 @@ void testSetLocale() {
1930
assertEquals("few", Messages.getInstance().getString("CloudQuantity.FEW"));
2031
assertEquals("ceiling varying between 5 and 15 feet", Messages.getInstance().getString("Remark.Ceiling.Height", 5, 15));
2132
}
33+
34+
@Test
35+
void testClearLocale() {
36+
Messages.getInstance().setLocale(Locale.FRENCH);
37+
assertEquals("peu", Messages.getInstance().getString("CloudQuantity.FEW"));
38+
Messages.getInstance().clearLocale();
39+
// After clearing, the JVM default locale is used; the key must still be resolvable.
40+
assertDoesNotThrow(() -> Messages.getInstance().getString("CloudQuantity.FEW"));
41+
}
42+
43+
@ParameterizedTest
44+
@ValueSource(strings = {"messages_de", "messages_es", "messages_fr", "messages_it",
45+
"messages_pl_PL", "messages_ru_RU", "messages_tr_TR", "messages_zh_CN"})
46+
@Disabled("Requires all locale bundles to be complete and up-to-date with the base bundle")
47+
void testLocaleContainsAllBaseKeys(final String bundleName) throws IOException {
48+
Properties base = loadProperties("internationalization/messages.properties");
49+
Properties locale = loadProperties("internationalization/" + bundleName + ".properties");
50+
Set<Object> baseKeys = base.keySet();
51+
for (Object key : baseKeys) {
52+
assertTrue(locale.containsKey(key),
53+
"Locale bundle '" + bundleName + "' is missing key: " + key);
54+
}
55+
}
56+
57+
private Properties loadProperties(final String resourcePath) throws IOException {
58+
Properties props = new Properties();
59+
try (InputStream is = getClass().getClassLoader().getResourceAsStream(resourcePath);
60+
InputStreamReader reader = new InputStreamReader(is, StandardCharsets.UTF_8)) {
61+
props.load(reader);
62+
}
63+
return props;
64+
}
2265
}

metarParser-entities/src/main/java/io/github/mivek/enums/Descriptive.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.github.mivek.enums;
22

33
import io.github.mivek.internationalization.Messages;
4+
import java.util.regex.Pattern;
45

56
/**
67
* Enumeration for descriptive. The first attribute is the code used in the
@@ -28,13 +29,16 @@ public enum Descriptive {
2829

2930
/** The descriptive's shortcut. */
3031
private final String shortcut;
32+
/** Pre-compiled pattern used to detect this descriptive in a weather token. */
33+
private final Pattern pattern;
3134

3235
/**
3336
* Constructor.
3437
* @param shortcut the shortcut of the descriptive.
3538
*/
3639
Descriptive(final String shortcut) {
3740
this.shortcut = shortcut;
41+
this.pattern = Pattern.compile("(" + shortcut + ")");
3842
}
3943

4044
/**
@@ -44,6 +48,15 @@ public String getShortcut() {
4448
return this.shortcut;
4549
}
4650

51+
/**
52+
* Returns the pre-compiled pattern used to match this descriptive in a weather token.
53+
*
54+
* @return the compiled {@link Pattern}.
55+
*/
56+
public Pattern getPattern() {
57+
return pattern;
58+
}
59+
4760
@Override
4861
public String toString() {
4962
return Messages.getInstance().getString("Descriptive." + shortcut);

metarParser-entities/src/main/java/io/github/mivek/enums/Phenomenon.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.github.mivek.enums;
22

33
import io.github.mivek.internationalization.Messages;
4+
import java.util.regex.Pattern;
45

56
/**
67
* Enumeration for phenomenon.
@@ -57,6 +58,8 @@ public enum Phenomenon {
5758

5859
/** Shortcut of the phenomenon. */
5960
private final String shortcut;
61+
/** Pre-compiled pattern used to match this phenomenon at the start of a weather token. */
62+
private final Pattern pattern;
6063

6164
/**
6265
* Constructor.
@@ -65,6 +68,7 @@ public enum Phenomenon {
6568
*/
6669
Phenomenon(final String shortcut) {
6770
this.shortcut = shortcut;
71+
this.pattern = Pattern.compile("^" + shortcut);
6872
}
6973

7074
@Override
@@ -80,4 +84,13 @@ public String toString() {
8084
public String getShortcut() {
8185
return shortcut;
8286
}
87+
88+
/**
89+
* Returns the pre-compiled pattern used to match this phenomenon at the start of a weather token.
90+
*
91+
* @return the compiled {@link Pattern}.
92+
*/
93+
public Pattern getPattern() {
94+
return pattern;
95+
}
8396
}

metarParser-parsers/src/main/java/io/github/mivek/parser/AbstractWeatherContainerParser.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ WeatherCondition parseWeatherCondition(final String weatherPart) {
6363
weatherPartCopy = weatherPartCopy.substring(i.getShortcut().length());
6464
}
6565
for (Descriptive des : Descriptive.values()) {
66-
if (Regex.findString(Pattern.compile("(" + des.getShortcut() + ")"), weatherPart) != null) {
66+
if (Regex.findString(des.getPattern(), weatherPart) != null) {
6767
wc.setDescriptive(des);
6868
weatherPartCopy = weatherPartCopy.substring(des.getShortcut().length());
6969
break;
@@ -74,7 +74,7 @@ WeatherCondition parseWeatherCondition(final String weatherPart) {
7474
while (!weatherPartCopy.isEmpty() && !weatherPartCopy.equals(previousToken)) {
7575
previousToken = weatherPartCopy;
7676
for (Phenomenon phenom: Phenomenon.values()) {
77-
if (Regex.find(Pattern.compile("^" + phenom.getShortcut()), weatherPartCopy)) {
77+
if (Regex.find(phenom.getPattern(), weatherPartCopy)) {
7878
wc.addPhenomenon(phenom);
7979
weatherPartCopy = weatherPartCopy.substring(phenom.getShortcut().length());
8080
}

metarParser-spi/src/main/java/io/github/mivek/provider/airport/impl/DefaultAirportProvider.java

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.io.InputStream;
88
import java.io.InputStreamReader;
99
import java.nio.charset.StandardCharsets;
10+
import java.util.Collections;
1011
import java.util.HashMap;
1112
import java.util.Map;
1213
import java.util.Objects;
@@ -23,28 +24,31 @@ public final class DefaultAirportProvider implements AirportProvider {
2324
private static final String AIRPORTS_RESOURCE = "data/airports.dat";
2425
private static final String COUNTRIES_RESOURCE = "data/countries.dat";
2526

26-
private volatile Map<String, Country> countries;
27-
private volatile Map<String, Airport> airports;
27+
/** Whether the data has been loaded. Set to true only after airports map is fully populated. */
28+
private volatile boolean initialized;
29+
/** Map of airports keyed by ICAO code. */
30+
private Map<String, Airport> airports;
2831

29-
/** private lock to avoid exposing the monitor. */
32+
/** Private lock to avoid exposing the monitor. */
3033
private final Object loadLock = new Object();
3134

32-
3335
/**
3436
* Ensure the airport and country data have been loaded.
3537
*
36-
* <p>This method is safe to call from multiple threads. It performs a double-checked
37-
* locking pattern using {@code loadLock} to initialize the data only once.
38+
* <p>This method is safe to call from multiple threads. It uses a double-checked
39+
* locking pattern on a single {@code volatile boolean} flag so that a thread
40+
* never observes a partially-initialized state.
3841
*/
3942
private void ensureLoaded() {
40-
if (airports != null && countries != null) {
43+
if (initialized) {
4144
return;
4245
}
4346
synchronized (loadLock) {
44-
if (airports != null && countries != null) {
47+
if (initialized) {
4548
return;
4649
}
4750
loadResources();
51+
initialized = true;
4852
}
4953
}
5054

@@ -98,22 +102,20 @@ private void loadResources() {
98102
throw new IllegalStateException(e);
99103
}
100104

101-
this.countries = localCountries;
102105
this.airports = localAirports;
103106
}
104107

105108
/**
106-
* Returns the map of loaded airports keyed by ICAO code.
109+
* Returns an unmodifiable view of the loaded airports keyed by ICAO code.
107110
*
108111
* <p>When this method is called the first time, it triggers loading of the underlying
109-
* country and airport resources. Subsequent calls return the cached map. The returned
110-
* map is the internal map instance (not a defensive copy).
112+
* country and airport resources. Subsequent calls return the cached map.
111113
*
112-
* @return the map of ICAO -> Airport
114+
* @return an unmodifiable map of ICAO -> Airport
113115
*/
114116
@Override
115117
public Map<String, Airport> getAirports() {
116118
ensureLoaded();
117-
return airports;
119+
return Collections.unmodifiableMap(airports);
118120
}
119121
}

metarParser-spi/src/main/java/io/github/mivek/provider/airport/impl/OurAirportsAirportProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.net.http.HttpResponse;
1414
import java.nio.charset.StandardCharsets;
1515
import java.time.Duration;
16+
import java.util.Collections;
1617
import java.util.HashMap;
1718
import java.util.Map;
1819
import org.apache.commons.csv.CSVFormat;
@@ -113,7 +114,7 @@ public void buildAirport() throws URISyntaxException, IOException, InterruptedEx
113114

114115
@Override
115116
public Map<String, Airport> getAirports() {
116-
return airports;
117+
return Collections.unmodifiableMap(airports);
117118
}
118119
}
119120

0 commit comments

Comments
 (0)