Accept i18n errors

common/app/com/commercetools/sunrise/common/controllers/SunriseFrameworkController.java

@@ -252,7 +252,7 @@ protected final void saveFormError(final Form<?> form, final String message) {
     }
 
     protected final void saveUnexpectedFormError(final Form<?> form, final Throwable throwable, final Logger logger) {
-        form.reject("Something went wrong, please try again"); // TODO i18n
+        form.reject("messages:defaultError");
         logger.error("The CTP request raised an unexpected exception", throwable);
     }
 }

common/app/com/commercetools/sunrise/common/template/engine/handlebars/PlayJavaFormResolver.java

@@ -53,7 +53,7 @@ private boolean isFalsy(@Nullable final String value) {
         return value != null && value.equals("false");
     }
 
-     ErrorsBean extractErrors(@Nullable final Form<?> form) {
+    ErrorsBean extractErrors(@Nullable final Form<?> form) {
         final ErrorsBean errorsBean = new ErrorsBean();
         final List<ErrorBean> errorList = new ArrayList<>();
         if (form != null) {

common/app/com/commercetools/sunrise/common/template/i18n/I18nResolver.java

@@ -17,10 +17,10 @@ public interface I18nResolver {
      * Resolves i18n message identified by a bundle and a key for the first found given locale.
      * @param locales the list of locales used to translate the message
      * @param i18nIdentifier identifier of the i18n message
-     * @param hashArgs list of hash arguments
+     * @param args list of named arguments
      * @return the resolved message in the first found given language, or absent if it could not be found
      */
-    Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs);
+    Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args);
 
     /**
      * Resolves i18n message identified by a bundle and a key for the first found given locale.
@@ -36,11 +36,11 @@ default Optional<String> get(final List<Locale> locales, final I18nIdentifier i1
      * Resolves i18n message identified by a bundle and a key for the first found given locale.
      * @param locales the list of locales used to translate the message
      * @param i18nIdentifier identifier of the i18n message
-     * @param hashArgs list of hash arguments
+     * @param args list of named arguments
      * @return the resolved message in the first found given language, or empty string if it could not be found
      */
-    default String getOrEmpty(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs) {
-        return get(locales, i18nIdentifier, hashArgs).orElse("");
+    default String getOrEmpty(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args) {
+        return get(locales, i18nIdentifier, args).orElse("");
     }
 
     /**
@@ -57,11 +57,11 @@ default String getOrEmpty(final List<Locale> locales, final I18nIdentifier i18nI
      * Resolves i18n message identified by a bundle and a key for the first found given locale.
      * @param locales the list of locales used to translate the message
      * @param i18nIdentifier identifier of the i18n message
-     * @param hashArgs list of hash arguments
+     * @param args list of named arguments
      * @return the resolved message in the first found given language, or the message key if it could not be found
      */
-    default String getOrKey(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs) {
-        return get(locales, i18nIdentifier, hashArgs).orElse(i18nIdentifier.messageKey());
+    default String getOrKey(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args) {
+        return get(locales, i18nIdentifier, args).orElse(i18nIdentifier.messageKey());
     }
 
     /**

common/app/com/commercetools/sunrise/common/template/i18n/composite/CompositeI18nResolver.java

@@ -22,9 +22,9 @@ private CompositeI18nResolver(final List<I18nResolver> i18nResolvers) {
     }
 
     @Override
-    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs) {
+    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args) {
         for (I18nResolver i18nResolver : i18nResolvers) {
-            final Optional<String> message = i18nResolver.get(locales, i18nIdentifier, hashArgs);
+            final Optional<String> message = i18nResolver.get(locales, i18nIdentifier, args);
             if (message.isPresent()) {
                 return message;
             }

common/app/com/commercetools/sunrise/common/template/i18n/yaml/YamlI18nResolver.java

@@ -40,11 +40,11 @@ private YamlI18nResolver(final String filepath, final List<Locale> locales, fina
     }
 
     @Override
-    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs) {
-        final String message = findPluralizedTranslation(locales, i18nIdentifier, hashArgs)
+    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args) {
+        final String message = findPluralizedTranslation(locales, i18nIdentifier, args)
                 .orElseGet(() -> findFirstTranslation(locales, i18nIdentifier.bundle(), i18nIdentifier.messageKey())
                         .orElse(null));
-        return Optional.ofNullable(message).map(resolvedValue -> replaceParameters(resolvedValue, hashArgs));
+        return Optional.ofNullable(message).map(resolvedValue -> replaceParameters(resolvedValue, args));
     }
 
     @Override
@@ -59,8 +59,8 @@ public static YamlI18nResolver of(final String filepath, final List<Locale> loca
     }
 
     private Optional<String> findPluralizedTranslation(final List<Locale> locales, final I18nIdentifier i18nIdentifier,
-                                                       final Map<String, Object> hashArgs) {
-        if (containsPlural(hashArgs)) {
+                                                       final Map<String, Object> args) {
+        if (containsPlural(args)) {
             final String pluralizedKey = i18nIdentifier.messageKey() + "_plural";
             return findFirstTranslation(locales, i18nIdentifier.bundle(), pluralizedKey);
         } else {
@@ -78,9 +78,9 @@ private Optional<String> findFirstTranslation(final List<Locale> locales, final
         return Optional.empty();
     }
 
-    private String replaceParameters(final String resolvedValue, final Map<String, Object> hashArgs) {
+    private String replaceParameters(final String resolvedValue, final Map<String, Object> args) {
         String message = StringUtils.defaultString(resolvedValue);
-        for (final Map.Entry<String, Object> entry : hashArgs.entrySet()) {
+        for (final Map.Entry<String, Object> entry : args.entrySet()) {
             if (entry.getValue() != null) {
                 final String parameter = "__" + entry.getKey() + "__";
                 message = message.replace(parameter, entry.getValue().toString());

common/app/com/commercetools/sunrise/common/utils/ErrorFormatter.java

@@ -3,8 +3,11 @@
 import com.google.inject.ImplementedBy;
 import play.data.validation.ValidationError;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
+import java.util.stream.IntStream;
 
 /**
  * Allows to format errors according to the available locales and error information.
@@ -16,18 +19,25 @@ public interface ErrorFormatter {
      * Formats the error message somehow, with the translation to the first available locale.
      * @param locales current given locales
      * @param message error message
+     * @param args list of named arguments
      * @return the error message localized and formatted
      */
-    String format(final List<Locale> locales, final String message);
+    String format(final List<Locale> locales, final String message, final Map<String, Object> args);
 
     /**
      * Formats the Play error message somehow, with the translation to the first available locale.
+     * As hash arguments, it defines the field key as "field" and all other arguments as its index, e.g. "0", "1", "2".
      * @param locales current given locales
      * @param error Play's validation error
      * @return the error message localized and formatted
      */
     default String format(final List<Locale> locales, final ValidationError error) {
-        final String message = format(locales, error.message());
-        return !error.key().isEmpty() ? message + ": " + error.key() : message;
+        final Map<String, Object> args = new HashMap<>();
+        if (error.key() != null && !error.key().isEmpty()) {
+            args.put("field", error.key());
+        }
+        IntStream.range(0, error.arguments().size())
+                .forEach(index -> args.put(String.valueOf(index), error.arguments().get(index)));
+        return format(locales, error.message(), args);
     }
 }

common/app/com/commercetools/sunrise/common/utils/ErrorFormatterImpl.java

@@ -7,17 +7,22 @@
 import javax.inject.Inject;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 
 class ErrorFormatterImpl implements ErrorFormatter {
 
+    private final I18nResolver i18nResolver;
+    private final I18nIdentifierFactory i18nIdentifierFactory;
+
     @Inject
-    private I18nResolver i18nResolver;
-    @Inject
-    private I18nIdentifierFactory i18nIdentifierFactory;
+    ErrorFormatterImpl(final I18nResolver i18nResolver, final I18nIdentifierFactory i18nIdentifierFactory) {
+        this.i18nResolver = i18nResolver;
+        this.i18nIdentifierFactory = i18nIdentifierFactory;
+    }
 
     @Override
-    public String format(final List<Locale> locales, final String messageKey) {
+    public String format(final List<Locale> locales, final String messageKey, final Map<String, Object> args) {
         final I18nIdentifier i18nIdentifier = i18nIdentifierFactory.create(messageKey);
-        return i18nResolver.getOrKey(locales, i18nIdentifier);
+        return i18nResolver.getOrKey(locales, i18nIdentifier, args);
     }
 }

common/test/com/commercetools/sunrise/common/template/engine/handlebars/PlayJavaFormResolverTest.java

@@ -2,22 +2,19 @@
 
 import com.commercetools.sunrise.common.forms.ErrorBean;
 import com.commercetools.sunrise.common.forms.ErrorsBean;
-import com.commercetools.sunrise.common.utils.ErrorFormatter;
-import com.google.common.collect.Maps;
 import org.junit.Test;
 import org.mockito.Mockito;
 import play.data.Form;
 import play.data.validation.ValidationError;
 
 import java.util.*;
-import java.util.function.Predicate;
 
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class PlayJavaFormResolverTest {
 
-    PlayJavaFormResolver formResolver = new PlayJavaFormResolver(singletonList(Locale.ENGLISH), (locales, message) ->
+    PlayJavaFormResolver formResolver = new PlayJavaFormResolver(singletonList(Locale.ENGLISH), (locales, message, args) ->
             message);
 
     @Test
@@ -28,14 +25,14 @@ public void extractErrors() throws Exception {
         ErrorsBean result = formResolver.extractErrors(form);
 
         List<ErrorBean> errors = result.getGlobalErrors();
-        checkError(errors.get(0), errorField1, "errorkey1", "errorMessage1");
-        checkError(errors.get(1), errorField1, "errorkey2", "errorMessage2");
-        checkError(errors.get(2), errorField2, "errorkey21", "errorMessage21");
+        checkError(errors.get(0), errorField1, "errorMessage1");
+        checkError(errors.get(1), errorField1, "errorMessage2");
+        checkError(errors.get(2), errorField2, "errorMessage21");
     }
 
-    private void checkError(ErrorBean error, String field, String key, String message) {
+    private void checkError(ErrorBean error, String field, String message) {
         assertThat(error.getField()).isEqualTo(field);
-        assertThat(error.getMessage()).isEqualTo(message + ": " + key);
+        assertThat(error.getMessage()).isEqualTo(message);
 
     }
 
@@ -50,4 +47,4 @@ private Form formWithSomeErrorsForFields(String field1, String field2) {
         Mockito.when(form.errors()).thenReturn(errorMap);
         return form;
     }
-}
+}

common/test/com/commercetools/sunrise/common/template/i18n/TestableI18nResolver.java

@@ -14,10 +14,10 @@ public TestableI18nResolver(final Map<String, String> i18nMap) {
     }
 
     @Override
-    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> hashArgs) {
+    public Optional<String> get(final List<Locale> locales, final I18nIdentifier i18nIdentifier, final Map<String, Object> args) {
         final String mapKey = String.format("%s/%s:%s", locales.get(0), i18nIdentifier.bundle(), i18nIdentifier.messageKey());
         final String message = i18nMap.get(mapKey);
-        final String parameters = hashArgs.entrySet().stream()
+        final String parameters = args.entrySet().stream()
                 .map(hashPair -> hashPair.getKey() + "=" + hashPair.getValue())
                 .collect(Collectors.joining(","));
         if (parameters.isEmpty()) {

common/test/com/commercetools/sunrise/common/utils/ErrorFormatterImplTest.java

@@ -0,0 +1,74 @@
+package com.commercetools.sunrise.common.utils;
+
+import com.commercetools.sunrise.common.template.i18n.I18nIdentifier;
+import com.commercetools.sunrise.common.template.i18n.I18nIdentifierFactory;
+import com.commercetools.sunrise.common.template.i18n.I18nResolver;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
+
+import static java.util.Collections.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ErrorFormatterImplTest {
+
+    private static final List<Locale> LOCALES = singletonList(Locale.ENGLISH);
+    private static final String MESSAGE_KEY = "error.required";
+    private static final I18nIdentifier I18N_IDENTIFIER = I18nIdentifier.of("main", MESSAGE_KEY);
+    private static final Map<String, Object> ARGS_WITH_FIELD = singletonMap("field", "username");
+
+    @Mock
+    private I18nResolver i18nResolver;
+    @Spy
+    private I18nIdentifierFactory i18nIdentifierFactory;
+
+    @InjectMocks
+    private ErrorFormatterImpl errorFormatter;
+
+    @Before
+    public void setUp() throws Exception {
+        when(i18nResolver.getOrKey(any(), any(), any())).thenCallRealMethod();
+    }
+
+    @Test
+    public void translatesMessageKey() throws Exception {
+        mockI18nResolverWithError();
+        final String errorMessage = errorFormatter.format(LOCALES, MESSAGE_KEY, emptyMap());
+        assertThat(errorMessage).isEqualTo("Required field");
+    }
+
+    @Test
+    public void returnsMessageKeyWhenNoMatch() throws Exception {
+        mockI18nResolverWithoutError();
+        final String errorMessage = errorFormatter.format(LOCALES, MESSAGE_KEY, emptyMap());
+        assertThat(errorMessage).isEqualTo(MESSAGE_KEY);
+    }
+
+    @Test
+    public void returnsMessageKeyWithFieldIfProvided() throws Exception {
+        mockI18nResolverWithError();
+        final String errorMessage = errorFormatter.format(LOCALES, MESSAGE_KEY, ARGS_WITH_FIELD);
+        assertThat(errorMessage).isEqualTo("Required field: username");
+    }
+
+    private void mockI18nResolverWithoutError() {
+        when(i18nResolver.get(any(), eq(I18N_IDENTIFIER), anyMap())).thenReturn(Optional.empty());
+    }
+
+    private void mockI18nResolverWithError() {
+        when(i18nResolver.get(any(), eq(I18N_IDENTIFIER), anyMap())).thenReturn(Optional.of("Required field"));
+        when(i18nResolver.get(any(), eq(I18N_IDENTIFIER), eq(singletonMap("field", "username")))).thenReturn(Optional.of("Required field: username"));
+    }
+}