Change TRANSLATION_CACHE implementation.

Author: VillePihlava

application/src/main/java/org/opentripplanner/gtfs/mapping/TranslationHelper.java

@@ -157,7 +157,7 @@ I18NString getTranslation(
           hm.put(translation.getLanguage(), translation.getTranslation());
         }
       }
-      return TranslatedString.getDeduplicatedI18NString(hm, false);
+      return TranslatedString.getI18NString(hm, false);
     }
     return NonLocalizedString.ofNullable(defaultValue);
   }

application/src/main/java/org/opentripplanner/netex/mapping/StationMapper.java

@@ -122,7 +122,7 @@ private I18NString resolveName(StopPlace stopPlace) {
         }
       }
 
-      name = TranslatedString.getDeduplicatedI18NString(translations, false);
+      name = TranslatedString.getI18NString(translations, false);
     } else {
       name = new NonLocalizedString(stopPlace.getName().getValue());
     }

application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java

@@ -529,10 +529,7 @@ public I18NString getAssumedName() {
       return null;
     }
     if (tags.containsKey("name")) {
-      return TranslatedString.getDeduplicatedI18NString(
-        this.generateI18NForPattern("{name}"),
-        false
-      );
+      return TranslatedString.getI18NString(this.generateI18NForPattern("{name}"), false);
     }
     if (tags.containsKey("otp:route_name")) {
       return new NonLocalizedString(tags.get("otp:route_name"));

application/src/main/java/org/opentripplanner/osm/wayproperty/NoteProperties.java

@@ -28,7 +28,7 @@ public StreetNoteAndMatcher generateNote(OsmEntity way) {
     if (PATTERN_MATCHER.matcher(notePattern).matches()) {
       //This gets language -> translation of notePattern and all tags (which can have translations name:en for example)
       Map<String, String> noteText = way.generateI18NForPattern(notePattern);
-      text = TranslatedString.getDeduplicatedI18NString(noteText, false);
+      text = TranslatedString.getI18NString(noteText, false);
     } else {
       text = LocalizedStringMapper.getInstance().map(notePattern, way);
     }

application/src/test/java/org/opentripplanner/framework/graphql/GraphQLUtilsTest.java

@@ -48,7 +48,7 @@ void testGetTranslationWithTranslations() {
       entry("en", "translationEn"),
       entry("fr", frenchTranslation)
     );
-    var translatedString = TranslatedString.getDeduplicatedI18NString(translations, false);
+    var translatedString = TranslatedString.getI18NString(translations, false);
 
     var translation = GraphQLUtils.getTranslation(translatedString, env);
 

domain-core/src/main/java/org/opentripplanner/core/model/i18n/TranslatedString.java

@@ -10,6 +10,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * This is for translated strings for which translations are read from OSM or GTFS alerts.
@@ -24,7 +25,8 @@ public class TranslatedString implements I18NString, Serializable {
    * Store all translations, so we don't get memory overhead for identical strings As this is
    * static, it isn't serialized when saving the graph.
    */
-  private static final HashMap<Map<String, String>, I18NString> TRANSLATION_CACHE = new HashMap<>();
+  private static final ConcurrentHashMap<Map<String, String>, I18NString> TRANSLATION_CACHE =
+    new ConcurrentHashMap<>();
 
   private final Map<String, String> translations = new HashMap<>();
 
@@ -52,72 +54,25 @@ public static I18NString getI18NString(String untranslated, String... translatio
     return getI18NString(map, false);
   }
 
-  /**
-   * Gets a deduplicated I18NString. If the translations only have a single value, return a
-   * NonLocalizedString, otherwise a TranslatedString. The resulting I18NString is interned for
-   * memory efficiency.
-   * <p>
-   * This should be used when calling this method during graph building. This should not be called
-   * from a real-time updater as this is not thread-safe and may cause a memory leak.
-   *
-   * @param translations A Map of languages and translations, a null language is the default
-   *                     translation
-   * @param forceTranslatedString Should the language information be kept, even when only a single
-   *                              translation is provided. This is useful when the language
-   *                              information is important or is presented to the user.
-   */
-  public static I18NString getDeduplicatedI18NString(
-    Map<String, String> translations,
-    boolean forceTranslatedString
-  ) {
-    return getI18NString(translations, true, forceTranslatedString);
-  }
-
-  /**
-   * Gets a non-deduplicated I18NString. If the translations only have a single value, return a
-   * NonLocalizedString, otherwise a TranslatedString. The resulting I18NString is NOT interned.
-   * <p>
-   * This should be used from real-time updaters to avoid memory leaks. For graph building, use
-   * {@link #getDeduplicatedI18NString(Map, boolean)} instead.
-   *
-   * @param translations A Map of languages and translations, a null language is the default
-   *                     translation
-   * @param forceTranslatedString Should the language information be kept, even when only a single
-   *                              translation is provided. This is useful when the language
-   *                              information is important or is presented to the user.
-   */
-  public static I18NString getI18NString(
-    Map<String, String> translations,
-    boolean forceTranslatedString
-  ) {
-    return getI18NString(translations, false, forceTranslatedString);
-  }
-
   /**
    * Gets an I18NString. If the translations only have a single value, return a NonLocalizedString,
    * otherwise a TranslatedString
    *
    * @param translations A Map of languages and translations, a null language is the default
    *                     translation
-   * @param intern Should the resulting I18NString be interned. This should be used when calling
-   *               this method during graph building. This should not be called from a real-time
-   *               updater as this is not thread-safe and may cause a memory leak.
    * @param forceTranslatedString Should the language information be kept, even when only a single
    *                              translation is provided. This is useful when the language
    *                              information is important or is presented to the user.
    */
-  static I18NString getI18NString(
+  public static I18NString getI18NString(
     Map<String, String> translations,
-    boolean intern,
     boolean forceTranslatedString
   ) {
-    if (translations.isEmpty()) {
-      throw new IllegalArgumentException("At least one translation must be provided");
-    }
-    if (TRANSLATION_CACHE.containsKey(translations)) {
-      return TRANSLATION_CACHE.get(translations);
-    } else {
-      I18NString ret;
+    I18NString ret = TRANSLATION_CACHE.get(translations);
+    if (ret == null) {
+      if (translations.isEmpty()) {
+        throw new IllegalArgumentException("At least one translation must be provided");
+      }
       // Check if we only have one name, even under multiple languages
       boolean allValuesEqual = new HashSet<>(translations.values()).size() == 1;
       var firstLanguage = translations.keySet().iterator().next();
@@ -130,11 +85,9 @@ static I18NString getI18NString(
       } else {
         ret = new TranslatedString(translations);
       }
-      if (intern) {
-        TRANSLATION_CACHE.put(translations, ret);
-      }
-      return ret;
+      TRANSLATION_CACHE.put(translations, ret);
     }
+    return ret;
   }
 
   @Override

domain-core/src/test/java/org/opentripplanner/framework/i18n/TranslatedStringTest.java

@@ -28,7 +28,7 @@ public void testGetI18NString() {
     assertTrue(string2 instanceof NonLocalizedString);
 
     translations.put("fi", "Testi");
-    I18NString string3 = TranslatedString.getDeduplicatedI18NString(translations, false);
+    I18NString string3 = TranslatedString.getI18NString(translations, false);
     assertEquals("Test", string3.toString());
     assertEquals("Test", string3.toString(Locale.ENGLISH));
     assertEquals("Testi", string3.toString(new Locale("fi")));
@@ -38,7 +38,7 @@ public void testGetI18NString() {
     translations2.put(null, "Test");
     translations2.put("en", "Test");
     translations2.put("fi", "Testi");
-    I18NString string4 = TranslatedString.getDeduplicatedI18NString(translations2, false);
+    I18NString string4 = TranslatedString.getI18NString(translations2, false);
     assertSame(string3, string4);
   }