From b46b160000d0a17ea0fc3d42088cac2082628643 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Thu, 4 Jul 2024 14:36:24 +0200 Subject: [PATCH 1/2] Fix duration parsing error handling --- .../LinearFunctionSerialization.java | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerialization.java b/src/main/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerialization.java index bccad0568c5..cb9dfb5c4cc 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerialization.java +++ b/src/main/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerialization.java @@ -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; @@ -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() {} /** @@ -61,11 +64,7 @@ public static Optional parse(String text, BiFunction 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)); } @@ -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. + *
+ * 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 + "'"); + } + } } From 4c5d1ea319a34d603f877e44cd222600b1853894 Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 5 Jul 2024 09:26:23 +0200 Subject: [PATCH 2/2] Added unit test --- .../framework/LinearFunctionSerializationTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerializationTest.java b/src/test/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerializationTest.java index e1785db61b1..46ef4b50e77 100644 --- a/src/test/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerializationTest.java +++ b/src/test/java/org/opentripplanner/routing/api/request/framework/LinearFunctionSerializationTest.java @@ -36,6 +36,8 @@ static Stream 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 """ ); } @@ -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())); @@ -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 fail() { return (a, b) -> Assertions.fail("Factory method called, not expected!"); }