Merge branch 'prefer-locale-param' into dev-2.x

Author: optionsome

application/src/main/java/org/opentripplanner/framework/graphql/GraphQLUtils.java

@@ -4,17 +4,60 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import org.opentripplanner.framework.i18n.I18NString;
 
+/**
+ * This class should always be used when translating fields or handling locales in GraphQL queries.
+ */
 public class GraphQLUtils {
 
-  public static String getTranslation(I18NString input, DataFetchingEnvironment environment) {
+  /**
+   * If input is {@code null}, return null. Otherwise, input is translated using a locale from this
+   * prioritized list:
+   * <ol>
+   *   <li>
+   *   {@code language} parameter of a queried field.
+   *   </li>
+   *   <li>
+   *   Locale from the DataFetchingEnvironment's local context (for journey planning queries this is {@code locale} parameter).
+   *   </li>
+   *   <li>
+   *   DataFetchingEnvironment's locale which comes from the accept-language header.
+   *   </li>
+   *   <li>
+   *   Default locale.
+   *   </li>
+   * </ol>
+   */
+  @Nullable
+  public static String getTranslation(
+    @Nullable I18NString input,
+    DataFetchingEnvironment environment
+  ) {
     if (input == null) {
       return null;
     }
     return input.toString(getLocale(environment));
   }
 
+  /**
+   * Returns locale from this prioritized list:
+   * <ol>
+   *   <li>
+   *   {@code language} parameter of a queried field.
+   *   </li>
+   *   <li>
+   *   Locale from the DataFetchingEnvironment's local context (for journey planning queries this is {@code locale} parameter).
+   *   </li>
+   *   <li>
+   *   DataFetchingEnvironment's locale which comes from the accept-language header.
+   *   </li>
+   *   <li>
+   *   Default locale.
+   *   </li>
+   * </ol>
+   */
   public static Locale getLocale(DataFetchingEnvironment environment) {
     var localeString = environment.getArgument("language");
     if (localeString != null) {
@@ -24,47 +67,90 @@ public static Locale getLocale(DataFetchingEnvironment environment) {
     return getLocaleFromEnvironment(environment);
   }
 
-  public static Locale getLocale(DataFetchingEnvironment environment, String localeString) {
+  /**
+   * Returns locale from this prioritized list:
+   * <ol>
+   *   <li>
+   *   {@code localeString}.
+   *   </li>
+   *   <li>
+   *   Locale from the DataFetchingEnvironment's local context (for journey planning queries this is {@code locale} parameter).
+   *   </li>
+   *   <li>
+   *   DataFetchingEnvironment's locale which comes from the accept-language header.
+   *   </li>
+   *   <li>
+   *   Default locale.
+   *   </li>
+   * </ol>
+   */
+  public static Locale getLocale(
+    DataFetchingEnvironment environment,
+    @Nullable String localeString
+  ) {
     if (localeString != null) {
       return Locale.forLanguageTag(localeString);
     }
 
     return getLocaleFromEnvironment(environment);
   }
 
-  public static Locale getLocale(DataFetchingEnvironment environment, Locale locale) {
+  /**
+   * Returns locale from this prioritized list:
+   * <ol>
+   *   <li>
+   *   {@code locale}.
+   *   </li>
+   *   <li>
+   *   Locale from the DataFetchingEnvironment's local context (for journey planning queries this is {@code locale} parameter).
+   *   </li>
+   *   <li>
+   *   DataFetchingEnvironment's locale which comes from the accept-language header.
+   *   </li>
+   *   <li>
+   *   Default locale.
+   *   </li>
+   * </ol>
+   */
+  public static Locale getLocale(DataFetchingEnvironment environment, @Nullable Locale locale) {
     if (locale != null) {
       return locale;
     }
 
     return getLocaleFromEnvironment(environment);
   }
 
+  /**
+   * Returns locale from this prioritized list:
+   * <ol>
+   *   <li>
+   *   Locale from the DataFetchingEnvironment's local context (for journey planning queries this is {@code locale} parameter).
+   *   </li>
+   *   <li>
+   *   DataFetchingEnvironment's locale which comes from the accept-language header.
+   *   </li>
+   *   <li>
+   *   Default locale.
+   *   </li>
+   * </ol>
+   */
   public static Locale getLocaleFromEnvironment(DataFetchingEnvironment environment) {
     // This can come from the accept-language header
-    var userLocale = environment.getLocale();
-    var defaultLocale = getDefaultLocale(environment);
-
-    if (userLocale == null) {
-      return defaultLocale.orElse(Locale.forLanguageTag("*"));
-    }
-
-    if (defaultLocale.isPresent() && acceptAnyLocale(userLocale)) {
-      return defaultLocale.get();
-    }
-
-    return userLocale;
+    var envLocale = environment.getLocale();
+    // This can come from a locale param
+    var localContextLocale = getLocalContextLocale(environment);
+    return localContextLocale.orElse(envLocale);
   }
 
-  private static Optional<Locale> getDefaultLocale(DataFetchingEnvironment environment) {
+  /**
+   * Returns locale from the DataFetchingEnvironment's local context (for journey planning queries
+   * this is {@code locale} parameter) or empty if none exist.
+   */
+  private static Optional<Locale> getLocalContextLocale(DataFetchingEnvironment environment) {
     Map<String, ?> localContext = environment.getLocalContext();
     if (localContext == null) {
       return Optional.empty();
     }
     return Optional.ofNullable((Locale) localContext.get("locale"));
   }
-
-  private static boolean acceptAnyLocale(Locale locale) {
-    return locale.getLanguage().equals("*");
-  }
 }

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

@@ -79,8 +79,7 @@ void testGetLocaleWithDefinedLocaleArg() {
   void testGetLocaleWithEnvLocale() {
     var frenchLocale = Locale.FRENCH;
     var env = DataFetchingEnvironmentImpl.newDataFetchingEnvironment(executionContext)
-      .localContext(Map.of("locale", Locale.GERMAN))
-      .locale(frenchLocale)
+      .locale(Locale.FRENCH)
       .build();
 
     var locale = GraphQLUtils.getLocale(env);
@@ -90,10 +89,11 @@ void testGetLocaleWithEnvLocale() {
 
   @Test
   void testGetLocaleWithLocalContextLocale() {
-    // Should use locale from local context if env locale is not defined
+    // Should use locale from local context even if env locale is defined
 
     var frenchLocale = Locale.FRENCH;
     var envWithNoLocale = DataFetchingEnvironmentImpl.newDataFetchingEnvironment(executionContext)
+      .locale(Locale.GERMAN)
       .localContext(Map.of("locale", Locale.FRENCH))
       .build();