Skip to content

Commit

Permalink
Merge pull request #5947 from entur/fix_duration_parsing_error_handling
Browse files Browse the repository at this point in the history
Fix duration parsing error handling
  • Loading branch information
vpaturet authored Jul 8, 2024
2 parents bd2eced + 4c5d1ea commit 967fe64
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.routing.api.request.framework;

import java.time.Duration;
import java.time.format.DateTimeParseException;
import java.util.Locale;
import java.util.Optional;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -39,6 +40,8 @@ public class LinearFunctionSerialization {
String.join(SEP, DUR, PLUS, NUM, VARIABLE)
);

private static final Pattern DECIMAL_NUMBER_PATTERN = Pattern.compile("\\d+(\\.\\d+)?");

private LinearFunctionSerialization() {}

/**
Expand All @@ -61,11 +64,7 @@ public static <T> Optional<T> parse(String text, BiFunction<Duration, Double, T>
var coefficient = Double.parseDouble(coefficientText);
coefficient = Units.normalizedFactor(coefficient, 0.0, 100.0);

// Unfortunately, to be backwards compatible we need to support decimal numbers.
// If a decimal number, then the value is converted to seconds
var constant = constantText.matches("\\d+(\\.\\d+)?")
? Duration.ofSeconds(IntUtils.round(Double.parseDouble(constantText)))
: DurationUtils.duration(constantText);
var constant = parseDecimalSecondsOrDuration(constantText);

return Optional.of(factory.apply(constant, coefficient));
}
Expand All @@ -85,4 +84,24 @@ public static String serialize(Duration constant, double coefficient) {
Units.factorToString(coefficient)
);
}

/**
* Parse a String as a Duration.
* Unfortunately, to be backwards compatible we need to support decimal numbers.
* If the text represents a decimal number, then the value is converted to seconds.
* <br/>
* The parsing function {@link DurationUtils#parseSecondsOrDuration(String)} cannot be used
* here since it supports only integer seconds, not decimal seconds.
*
*/
private static Duration parseDecimalSecondsOrDuration(String text) {
try {
if (DECIMAL_NUMBER_PATTERN.matcher(text).matches()) {
return Duration.ofSeconds(IntUtils.round(Double.parseDouble(text)));
}
return DurationUtils.duration(text);
} catch (DateTimeParseException e) {
throw new IllegalArgumentException("Unable to parse duration: '" + text + "'");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ static Stream<Arguments> parseTestCases() {
3h + 5.111 t || 3h | 5.1
7m + 10.1 x || 7m | 10.0
PT7s + 10.1 x || 7s | 10.0
0.1 + 10.1 x || 0s | 10.0
0.5 + 10.1 x || 1s | 10.0
"""
);
}
Expand All @@ -53,7 +55,7 @@ void parseTest(String input, String expectedConstant, double expectedCoefficient
}

@Test
void parseEmtpy() {
void parseEmpty() {
assertEquals(Optional.empty(), LinearFunctionSerialization.parse(null, fail()));
assertEquals(Optional.empty(), LinearFunctionSerialization.parse("", fail()));
assertEquals(Optional.empty(), LinearFunctionSerialization.parse(" \r\n", fail()));
Expand All @@ -77,6 +79,15 @@ void parseIllegalArgument() {
assertEquals("Unable to parse function: 'foo'", ex.getMessage());
}

@Test
void parseIllegalDuration() {
var ex = assertThrows(
IllegalArgumentException.class,
() -> LinearFunctionSerialization.parse("600ss + 1.3 t", fail())
);
assertEquals("Unable to parse duration: '600ss'", ex.getMessage());
}

private static BiFunction<Duration, Double, ?> fail() {
return (a, b) -> Assertions.fail("Factory method called, not expected!");
}
Expand Down

0 comments on commit 967fe64

Please sign in to comment.