Skip to content

Commit

Permalink
ICU-22894 MF2, ICU4J: implements configuring the error handling behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
mihnita authored and Squash Bot committed Sep 19, 2024
1 parent 5991c93 commit fbbbc73
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public FormattedPlaceholder format(Object toFormat, Map<String, Object> variable
toFormat, new PlainStringFormattedValue("{|" + toFormat + "|}"));
}
} else if (toFormat instanceof Temporal) {
toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat);
toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat);
}
// Not an else-if here, because the `Temporal` conditions before make `toFormat` a `Calendar`
if (toFormat instanceof Calendar) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.ibm.icu.message2.MFDataModel.StringPart;
import com.ibm.icu.message2.MFDataModel.VariableRef;
import com.ibm.icu.message2.MFDataModel.Variant;
import com.ibm.icu.message2.MessageFormatter.ErrorHandlingBehavior;
import com.ibm.icu.util.Calendar;
import com.ibm.icu.util.CurrencyAmount;

Expand All @@ -39,15 +40,21 @@
// TODO: move this in the MessageFormatter?
class MFDataModelFormatter {
private final Locale locale;
private final ErrorHandlingBehavior errorHandlingBehavior;
private final MFDataModel.Message dm;

private final MFFunctionRegistry standardFunctions;
private final MFFunctionRegistry customFunctions;
private static final MFFunctionRegistry EMPTY_REGISTY = MFFunctionRegistry.builder().build();

MFDataModelFormatter(
MFDataModel.Message dm, Locale locale, MFFunctionRegistry customFunctionRegistry) {
MFDataModel.Message dm,
Locale locale,
ErrorHandlingBehavior errorHandlingBehavior,
MFFunctionRegistry customFunctionRegistry) {
this.locale = locale;
this.errorHandlingBehavior = errorHandlingBehavior == null
? ErrorHandlingBehavior.BEST_EFFORT : errorHandlingBehavior;
this.dm = dm;
this.customFunctions =
customFunctionRegistry == null ? EMPTY_REGISTY : customFunctionRegistry;
Expand Down Expand Up @@ -94,21 +101,23 @@ String format(Map<String, Object> arguments) {
if (dm instanceof MFDataModel.PatternMessage) {
MFDataModel.PatternMessage pm = (MFDataModel.PatternMessage) dm;
variables = resolveDeclarations(pm.declarations, arguments);
if (pm.pattern == null) {
fatalFormattingError("The PatternMessage is null.");
}
patternToRender = pm.pattern;
} else if (dm instanceof MFDataModel.SelectMessage) {
MFDataModel.SelectMessage sm = (MFDataModel.SelectMessage) dm;
variables = resolveDeclarations(sm.declarations, arguments);
patternToRender = findBestMatchingPattern(sm, variables, arguments);
if (patternToRender == null) {
fatalFormattingError("Cannor find a match for the selector.");
}
} else {
fatalFormattingError("Unknown message type.");
// formattingError throws, so the return does not actually happen
return "ERROR!";
}

if (patternToRender == null) {
return "ERROR!";
}

StringBuilder result = new StringBuilder();
for (MFDataModel.PatternPart part : patternToRender.parts) {
if (part instanceof MFDataModel.StringPart) {
Expand Down Expand Up @@ -175,15 +184,15 @@ private Pattern findBestMatchingPattern(
// spec: Append `rv` as the last element of the list `res`.
res.add(rs);
} else {
throw new IllegalArgumentException("Unknown selector type: " + functionName);
fatalFormattingError("Unknown selector type: " + functionName);
}
}

// This should not be possible, we added one function for each selector,
// or we have thrown an exception.
// But just in case someone removes the throw above?
if (res.size() != selectors.size()) {
throw new IllegalArgumentException(
fatalFormattingError(
"Something went wrong, not enough selector functions, "
+ res.size() + " vs. " + selectors.size());
}
Expand Down Expand Up @@ -322,8 +331,7 @@ private Pattern findBestMatchingPattern(
// And should do that only once, when building the data model.
if (patternToRender == null) {
// If there was a case with all entries in the keys `*` this should not happen
throw new IllegalArgumentException(
"The selection went wrong, cannot select any option.");
fatalFormattingError("The selection went wrong, cannot select any option.");
}

return patternToRender;
Expand Down Expand Up @@ -394,7 +402,7 @@ public ResolvedSelector(
}
}

private static void fatalFormattingError(String message) {
private static void fatalFormattingError(String message) throws IllegalArgumentException {
throw new IllegalArgumentException(message);
}

Expand All @@ -412,7 +420,7 @@ private FormatterFactory getFormattingFunctionFactoryByName(
functionName = customFunctions.getDefaultFormatterNameForType(clazz);
}
if (functionName == null) {
throw new IllegalArgumentException(
fatalFormattingError(
"Object to format without a function, and unknown type: "
+ toFormat.getClass().getName());
}
Expand Down Expand Up @@ -528,11 +536,17 @@ private FormattedPlaceholder formatExpression(

FormatterFactory funcFactory = getFormattingFunctionFactoryByName(toFormat, functionName);
if (funcFactory == null) {
if (errorHandlingBehavior == ErrorHandlingBehavior.STRICT) {
fatalFormattingError("unable to find function at " + fallbackString);
}
return new FormattedPlaceholder(expression, new PlainStringFormattedValue(fallbackString));
}
Formatter ff = funcFactory.createFormatter(locale, options);
String res = ff.formatToString(toFormat, arguments);
if (res == null) {
if (errorHandlingBehavior == ErrorHandlingBehavior.STRICT) {
fatalFormattingError("unable to format string at " + fallbackString);
}
res = fallbackString;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,15 @@
public class MessageFormatter {
private final Locale locale;
private final String pattern;
private final ErrorHandlingBehavior errorHandlingBehavior;
private final MFFunctionRegistry functionRegistry;
private final MFDataModel.Message dataModel;
private final MFDataModelFormatter modelFormatter;

private MessageFormatter(Builder builder) {
this.locale = builder.locale;
this.functionRegistry = builder.functionRegistry;
this.errorHandlingBehavior = builder.errorHandlingBehavior;
if ((builder.pattern == null && builder.dataModel == null)
|| (builder.pattern != null && builder.dataModel != null)) {
throw new IllegalArgumentException(
Expand All @@ -171,7 +173,7 @@ private MessageFormatter(Builder builder) {
+ "Error: " + pe.getMessage() + "\n");
}
}
modelFormatter = new MFDataModelFormatter(dataModel, locale, functionRegistry);
modelFormatter = new MFDataModelFormatter(dataModel, locale, errorHandlingBehavior, functionRegistry);
}

/**
Expand Down Expand Up @@ -201,6 +203,20 @@ public Locale getLocale() {
return locale;
}

/**
* Get the {@link ErrorHandlingBehavior} to use when encountering errors in
* the current {@code MessageFormatter}.
*
* @return the error handling behavior.
*
* @internal ICU 76 technology preview
* @deprecated This API is for technology preview only.
*/
@Deprecated
public ErrorHandlingBehavior getErrorHandlingBehavior() {
return errorHandlingBehavior;
}

/**
* Get the pattern (the serialized message in MessageFormat 2 syntax) of
* the current {@code MessageFormatter}.
Expand Down Expand Up @@ -271,6 +287,37 @@ public FormattedMessage format(Map<String, Object> arguments) {
throw new RuntimeException("Not yet implemented.");
}

/**
* Determines how the formatting errors will be handled at runtime.
*
* <p>Parsing errors and data model errors always throw and will not be affected by this setting.<br>
* But resolution errors and formatting errors will either try to fallback (if possible) or throw,
* depending on this setting.</p>
*
* <p>Used in conjunction with the
* {@link MessageFormatter.Builder#setErrorHandlingBehavior(ErrorHandlingBehavior)} method.</p>
*
* @internal ICU 76 technology preview
* @deprecated This API is for technology preview only.
*/
@Deprecated
public static enum ErrorHandlingBehavior {
/**
* Suppress errors and return best-effort output.
*
* @internal ICU 76 technology preview
* @deprecated This API is for technology preview only.
*/
BEST_EFFORT,
/**
* Signal all {@code MessageFormat} errors by throwing a {@link RuntimeException}.
*
* @internal ICU 76 technology preview
* @deprecated This API is for technology preview only.
*/
STRICT
}

/**
* A {@code Builder} used to build instances of {@link MessageFormatter}.
*
Expand All @@ -281,6 +328,7 @@ public FormattedMessage format(Map<String, Object> arguments) {
public static class Builder {
private Locale locale = Locale.getDefault(Locale.Category.FORMAT);
private String pattern = null;
private ErrorHandlingBehavior errorHandlingBehavior = ErrorHandlingBehavior.BEST_EFFORT;
private MFFunctionRegistry functionRegistry = MFFunctionRegistry.builder().build();
private MFDataModel.Message dataModel = null;

Expand Down Expand Up @@ -319,6 +367,23 @@ public Builder setPattern(String pattern) {
return this;
}

/**
* Sets the {@link ErrorHandlingBehavior} to use when encountering errors at formatting time.
*
* <p>The default value is {@code ErrorHandlingBehavior.BEST_EFFORT}, trying to fallback.</p>
*
* @param the error handling behavior to use.
* @return the builder, for fluent use.
*
* @internal ICU 76 technology preview
* @deprecated This API is for technology preview only.
*/
@Deprecated
public Builder setErrorHandlingBehavior(ErrorHandlingBehavior errorHandlingBehavior) {
this.errorHandlingBehavior = errorHandlingBehavior;
return this;
}

/**
* Sets an instance of {@link MFFunctionRegistry} that should register any
* custom functions used by the message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -574,4 +574,85 @@ public void testVariableOptionsInSelectorWithLocalVar() {
assertEquals("test local vars loop", "Count = 23, OffCount = 21, and delta=2.",
mfVar2.formatToString(Args.of("count", 23, "delta", 2)));
}

// Needs more tests. Ported from the equivalent test in ICU4C
@Test
public void testFormatterAPI() {
String result;
Map<String, Object> messageArguments = new HashMap<>();

// Check that constructing the formatter fails
// if there's a syntax error
String pattern = "{{}";
MessageFormatter.Builder mfBuilder = MessageFormatter.builder();
MessageFormatter mf;
try {
mf = mfBuilder
// This shouldn't matter, since there's a syntax error
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
.setPattern(pattern)
.build();
errln("error expected");
} catch (IllegalArgumentException e) {
assertTrue("", e.getMessage().contains("Parse error"));
}

/*
Parsing is done when setPattern() is called,
so setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT) or setSuppressErrors must be called
_before_ setPattern() to get the right behavior,
and if either method is called after setting a pattern,
setPattern() has to be called again.
*/

// Should get the same behavior with strict errors
try {
mf = mfBuilder.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
// Force re-parsing, as above comment
.setPattern(pattern)
.build();
errln("error expected");
} catch (IllegalArgumentException e) {
assertTrue("", e.getMessage().contains("Parse error"));
}

// Try the same thing for a pattern with a resolution error
pattern = "{{{$x}}}";
// Check that a pattern with a resolution error gives fallback output
mf = mfBuilder
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
.setPattern(pattern)
.build();
result = mf.formatToString(messageArguments);
assertEquals("", "{$x}", result);

try {
// Check that we do get an error with strict errors
mf = mfBuilder
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
.build();
// U_ASSERT(U_SUCCESS(errorCode));
result = mf.formatToString(messageArguments);
errln("error expected");
} catch (IllegalArgumentException e) {
assertTrue("", e.getMessage().contains("unable to find function"));
}

// Finally, check a valid pattern
pattern = "hello";
mf = mfBuilder
.setPattern(pattern)
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
.build();
result = mf.formatToString(messageArguments);
assertEquals("", "hello", result);

// Check that behavior is the same with strict errors
mf = mfBuilder
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
.build();
result = mf.formatToString(messageArguments);
assertEquals("", "hello", result);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ static void rewriteDates(Param[] params) {
// "params": [{"name": "exp"}, { "value": { "date": 1722746637000 } }]
for (int i = 0; i < params.length; i++) {
Param pair = params[i];
if (pair.value instanceof Map) {
Map innerMap = (Map) pair.value;
if (pair.value instanceof Map<?, ?>) {
@SuppressWarnings("unchecked")
Map<String, Object> innerMap = (Map<String, Object>) pair.value;
if (innerMap.size() == 1 && innerMap.containsKey("date") && innerMap.get("date") instanceof Double) {
Long dateValue = Double.valueOf((Double) innerMap.get("date")).longValue();
params[i] = new Param(pair.name, new Date(dateValue));
Expand All @@ -103,8 +104,9 @@ static void rewriteDecimals(Param[] params) {
// "params": [{"name": "val"}, {"value": {"decimal": "1234567890123456789.987654321"}}]
for (int i = 0; i < params.length; i++) {
Param pair = params[i];
if (pair.value instanceof Map) {
Map innerMap = (Map) pair.value;
if (pair.value instanceof Map<?, ?>) {
@SuppressWarnings("unchecked")
Map<String, Object> innerMap = (Map<String, Object>) pair.value;
if (innerMap.size() == 1 && innerMap.containsKey("decimal")
&& innerMap.get("decimal") instanceof String) {
String decimalValue = (String) innerMap.get("decimal");
Expand Down

0 comments on commit fbbbc73

Please sign in to comment.