Add validation of routeSortOrder

ibi-group · Mar 22, 2024 · aa8efd2 · aa8efd2
1 parent ccc95e6
commit aa8efd2
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 2 deletions.
diff --git a/src/main/java/org/opentripplanner/framework/lang/IntUtils.java b/src/main/java/org/opentripplanner/framework/lang/IntUtils.java
@@ -104,6 +104,19 @@ public static int requireInRange(int value, int minInclusive, int maxInclusive)
     return requireInRange(value, minInclusive, maxInclusive, null);
   }
 
+  /**
+   * Check if the given {@code value} is null or a non-negative integer.
+   *
+   * @throws IllegalArgumentException
+   */
+  public static Integer requireNullOrNotNegative(Integer value, String field) {
+    if (value == null) {
+      return null;
+    } else {
+      return requireNotNegative(value, field);
+    }
+  }
+
   public static int requireNotNegative(int value, String field) {
     if (value < 0) {
       throw new IllegalArgumentException(

diff --git a/src/main/java/org/opentripplanner/transit/model/network/Route.java b/src/main/java/org/opentripplanner/transit/model/network/Route.java
@@ -10,6 +10,7 @@
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opentripplanner.framework.i18n.I18NString;
+import org.opentripplanner.framework.lang.IntUtils;
 import org.opentripplanner.transit.model.basic.SubMode;
 import org.opentripplanner.transit.model.basic.TransitMode;
 import org.opentripplanner.transit.model.framework.AbstractTransitEntity;
@@ -55,7 +56,7 @@ public final class Route extends AbstractTransitEntity<Route, RouteBuilder> impl
     this.branding = builder.getBranding();
     this.groupsOfRoutes = listOfNullSafe(builder.getGroupsOfRoutes());
     this.gtfsType = builder.getGtfsType();
-    this.gtfsSortOrder = builder.getGtfsSortOrder();
+    this.gtfsSortOrder = IntUtils.requireNullOrNotNegative(builder.getGtfsSortOrder(), "sortOrder");
     this.netexSubmode = SubMode.getOrBuildAndCacheForever(builder.getNetexSubmode());
     this.flexibleLineType = builder.getFlexibleLineType();
     this.description = builder.getDescription();
@@ -148,6 +149,12 @@ public Integer getGtfsType() {
     return gtfsType;
   }
 
+  /**
+   * The visual sort priority of this route when displayed in a graphical interface.
+   * A lower number means that the route has a higher priority.
+   * <p>
+   * Pass-through information from GTFS. This information is not used by OTP.
+   */
   @Nullable
   public Integer getGtfsSortOrder() {
     return gtfsSortOrder;

diff --git a/src/test/java/org/opentripplanner/framework/lang/IntUtilsTest.java b/src/test/java/org/opentripplanner/framework/lang/IntUtilsTest.java
@@ -2,13 +2,15 @@
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.opentripplanner.framework.lang.IntUtils.concat;
 import static org.opentripplanner.framework.lang.IntUtils.intArray;
 import static org.opentripplanner.framework.lang.IntUtils.intArrayToString;
 import static org.opentripplanner.framework.lang.IntUtils.intToString;
 import static org.opentripplanner.framework.lang.IntUtils.requireInRange;
 import static org.opentripplanner.framework.lang.IntUtils.requireNotNegative;
+import static org.opentripplanner.framework.lang.IntUtils.requireNullOrNotNegative;
 import static org.opentripplanner.framework.lang.IntUtils.shiftArray;
 import static org.opentripplanner.framework.lang.IntUtils.standardDeviation;
 
@@ -102,6 +104,14 @@ void testRequireNotNegative() {
     assertEquals("Negative value not expected for value: -1", ex.getMessage());
   }
 
+  @Test
+  void testRequireNotNegativeOrNull() {
+    assertNull(requireNullOrNotNegative(null, "ok"));
+    assertEquals(5, requireNullOrNotNegative(5, "ok"));
+    assertEquals(0, requireNullOrNotNegative(0, "ok"));
+    assertThrows(IllegalArgumentException.class, () -> requireNullOrNotNegative(-5, "ok"));
+  }
+
   @Test
   void testRequireInRange() {
     // OK

diff --git a/src/test/java/org/opentripplanner/transit/model/network/RouteTest.java b/src/test/java/org/opentripplanner/transit/model/network/RouteTest.java
@@ -128,7 +128,7 @@ void sameAs() {
     assertFalse(subject.sameAs(subject.copy().withColor("X").build()));
     assertFalse(subject.sameAs(subject.copy().withTextColor("X").build()));
     assertFalse(subject.sameAs(subject.copy().withGtfsType(-1).build()));
-    assertFalse(subject.sameAs(subject.copy().withGtfsSortOrder(-1).build()));
+    assertFalse(subject.sameAs(subject.copy().withGtfsSortOrder(99).build()));
     assertFalse(subject.sameAs(subject.copy().withFlexibleLineType("X").build()));
     assertFalse(subject.sameAs(subject.copy().withUrl("X").build()));
   }