Merge remote-tracking branch 'refs/remotes/entur/check_timerange_valu…

…e' into otp2_entur_develop
entur · May 23, 2024 · 58d4882 · 58d4882
2 parents f871f99 + 305c6ce
commit 58d4882
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 7 deletions.
diff --git a/src/main/java/org/opentripplanner/apis/transmodel/model/stop/QuayType.java b/src/main/java/org/opentripplanner/apis/transmodel/model/stop/QuayType.java
@@ -1,5 +1,7 @@
 package org.opentripplanner.apis.transmodel.model.stop;
 
+import static org.opentripplanner.apis.transmodel.support.GqlUtil.getPositiveNonNullIntegerArgument;
+
 import graphql.Scalars;
 import graphql.schema.GraphQLArgument;
 import graphql.schema.GraphQLFieldDefinition;
@@ -214,6 +216,9 @@ public static GraphQLObjectType create(
             GraphQLArgument
               .newArgument()
               .name("timeRange")
+              .description(
+                "Duration in seconds from start time to search forward for estimated calls. Must be a positive value. Default value is 24 hours"
+              )
               .type(Scalars.GraphQLInt)
               .defaultValue(24 * 60 * 60)
               .build()
@@ -299,8 +304,8 @@ public static GraphQLObjectType create(
             Integer departuresPerLineAndDestinationDisplay = environment.getArgument(
               "numberOfDeparturesPerLineAndDestinationDisplay"
             );
-            Integer timeRangeInput = environment.getArgument("timeRange");
-            Duration timeRange = Duration.ofSeconds(timeRangeInput.longValue());
+            int timeRangeInput = getPositiveNonNullIntegerArgument(environment, "timeRange");
+            Duration timeRange = Duration.ofSeconds(timeRangeInput);
             StopLocation stop = environment.getSource();
 
             JourneyWhiteListed whiteListed = new JourneyWhiteListed(environment);

diff --git a/src/main/java/org/opentripplanner/apis/transmodel/model/stop/StopPlaceType.java b/src/main/java/org/opentripplanner/apis/transmodel/model/stop/StopPlaceType.java
@@ -1,6 +1,7 @@
 package org.opentripplanner.apis.transmodel.model.stop;
 
 import static java.lang.Boolean.TRUE;
+import static org.opentripplanner.apis.transmodel.support.GqlUtil.getPositiveNonNullIntegerArgument;
 
 import graphql.Scalars;
 import graphql.schema.DataFetchingEnvironment;
@@ -289,6 +290,9 @@ public static GraphQLObjectType create(
             GraphQLArgument
               .newArgument()
               .name("timeRange")
+              .description(
+                "Duration in seconds from start time to search forward for estimated calls. Must be a positive value. Default value is 24 hours"
+              )
               .type(Scalars.GraphQLInt)
               .defaultValue(24 * 60 * 60)
               .build()
@@ -356,8 +360,8 @@ public static GraphQLObjectType create(
             Integer departuresPerLineAndDestinationDisplay = environment.getArgument(
               "numberOfDeparturesPerLineAndDestinationDisplay"
             );
-            Integer timeRangeInput = environment.getArgument("timeRange");
-            Duration timeRage = Duration.ofSeconds(timeRangeInput.longValue());
+            int timeRangeInput = getPositiveNonNullIntegerArgument(environment, "timeRange");
+            Duration timeRange = Duration.ofSeconds(timeRangeInput);
 
             MonoOrMultiModalStation monoOrMultiModalStation = environment.getSource();
             JourneyWhiteListed whiteListed = new JourneyWhiteListed(environment);
@@ -374,7 +378,7 @@ public static GraphQLObjectType create(
                 getTripTimesForStop(
                   singleStop,
                   startTime,
-                  timeRage,
+                  timeRange,
                   arrivalDeparture,
                   includeCancelledTrips,
                   numberOfDepartures,
@@ -419,7 +423,7 @@ public static GraphQLObjectType create(
   public static Stream<TripTimeOnDate> getTripTimesForStop(
     StopLocation stop,
     Instant startTimeSeconds,
-    Duration timeRage,
+    Duration timeRange,
     ArrivalDeparture arrivalDeparture,
     boolean includeCancelledTrips,
     int numberOfDepartures,
@@ -434,7 +438,7 @@ public static Stream<TripTimeOnDate> getTripTimesForStop(
     List<StopTimesInPattern> stopTimesInPatterns = transitService.stopTimesForStop(
       stop,
       startTimeSeconds,
-      timeRage,
+      timeRange,
       numberOfDepartures,
       arrivalDeparture,
       includeCancelledTrips

diff --git a/src/main/java/org/opentripplanner/apis/transmodel/support/GqlUtil.java b/src/main/java/org/opentripplanner/apis/transmodel/support/GqlUtil.java
@@ -103,6 +103,25 @@ public static boolean hasArgument(DataFetchingEnvironment environment, String na
     return environment.containsArgument(name) && environment.getArgument(name) != null;
   }
 
+  /**
+   * Return the integer value of the argument or throw an exception if the value is null
+   * or strictly negative.
+   * This should generally be handled at the GraphQL schema level,
+   * but must sometimes be implemented programmatically to preserve backward compatibility.
+   */
+  public static int getPositiveNonNullIntegerArgument(
+    DataFetchingEnvironment environment,
+    String argumentName
+  ) {
+    Integer argumentValue = environment.getArgument(argumentName);
+    if (argumentValue == null || argumentValue < 0) {
+      throw new IllegalArgumentException(
+        "The argument '" + argumentName + "' should be a non-null positive value: " + argumentValue
+      );
+    }
+    return argumentValue;
+  }
+
   public static <T> List<T> listOfNullSafe(T element) {
     return element == null ? List.of() : List.of(element);
   }

diff --git a/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql b/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql
@@ -571,6 +571,7 @@ type Quay implements PlaceInterface {
     omitNonBoarding: Boolean = false @deprecated(reason : "Non-functional. Use arrivalDeparture instead."),
     "DateTime for when to fetch estimated calls from. Default value is current time"
     startTime: DateTime,
+    "Duration in seconds from start time to search forward for estimated calls. Must be a positive value. Default value is 24 hours"
     timeRange: Int = 86400,
     "Parameters for indicating the only authorities and/or lines or quays to list estimatedCalls for"
     whiteListed: InputWhiteListed,
@@ -1134,6 +1135,7 @@ type StopPlace implements PlaceInterface {
     numberOfDeparturesPerLineAndDestinationDisplay: Int,
     "DateTime for when to fetch estimated calls from. Default value is current time"
     startTime: DateTime,
+    "Duration in seconds from start time to search forward for estimated calls. Must be a positive value. Default value is 24 hours"
     timeRange: Int = 86400,
     "Parameters for indicating the only authorities and/or lines or quays to list estimatedCalls for"
     whiteListed: InputWhiteListed,

diff --git a/src/test/java/org/opentripplanner/apis/transmodel/support/GqlUtilTest.java b/src/test/java/org/opentripplanner/apis/transmodel/support/GqlUtilTest.java
@@ -2,18 +2,22 @@
 
 import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import graphql.ExecutionInput;
 import graphql.execution.ExecutionContext;
 import graphql.execution.ExecutionId;
+import graphql.schema.DataFetchingEnvironment;
 import graphql.schema.DataFetchingEnvironmentImpl;
+import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
 
 public class GqlUtilTest {
 
   static final ExecutionContext executionContext;
+  private static final String TEST_ARGUMENT = "testArgument";
 
   static {
     ExecutionInput executionInput = ExecutionInput
@@ -29,6 +33,54 @@ public class GqlUtilTest {
         .build();
   }
 
+  @Test
+  void testGetPositiveNonNullIntegerArgumentWithStrictlyPositiveValue() {
+    var env = buildEnvWithTestValue(1);
+    assertEquals(1, GqlUtil.getPositiveNonNullIntegerArgument(env, TEST_ARGUMENT));
+  }
+
+  @Test
+  void testGetPositiveNonNullIntegerArgumentWithZeroValue() {
+    var env = buildEnvWithTestValue(0);
+    assertEquals(0, GqlUtil.getPositiveNonNullIntegerArgument(env, TEST_ARGUMENT));
+  }
+
+  @Test
+  void testGetPositiveNonNullIntegerArgumentWithNegativeValue() {
+    var env = buildEnvWithTestValue(-1);
+    assertThrows(
+      IllegalArgumentException.class,
+      () -> GqlUtil.getPositiveNonNullIntegerArgument(env, TEST_ARGUMENT)
+    );
+  }
+
+  @Test
+  void testGetPositiveNonNullIntegerArgumentWithNullValue() {
+    var env = buildEnvWithTestValue(null);
+    assertThrows(
+      IllegalArgumentException.class,
+      () -> GqlUtil.getPositiveNonNullIntegerArgument(env, TEST_ARGUMENT)
+    );
+  }
+
+  @Test
+  void testGetPositiveNonNullIntegerArgumentWithoutValue() {
+    var env = DataFetchingEnvironmentImpl.newDataFetchingEnvironment(executionContext).build();
+    assertThrows(
+      IllegalArgumentException.class,
+      () -> GqlUtil.getPositiveNonNullIntegerArgument(env, TEST_ARGUMENT)
+    );
+  }
+
+  private static DataFetchingEnvironment buildEnvWithTestValue(Integer value) {
+    Map<String, Object> argsMap = new HashMap<>();
+    argsMap.put(TEST_ARGUMENT, value);
+    return DataFetchingEnvironmentImpl
+      .newDataFetchingEnvironment(executionContext)
+      .arguments(argsMap)
+      .build();
+  }
+
   @Test
   void testGetLocaleWithLangArgument() {
     var env = DataFetchingEnvironmentImpl