Merge pull request opentripplanner#7331 from ibi-group/transfer-count

Ensure valid values for `FareLegRule#transferCount`

Author: leonardehrenfried

application/src/ext/java/org/opentripplanner/ext/fares/model/FareTransferRule.java

@@ -8,6 +8,7 @@
 import javax.annotation.Nullable;
 import org.opentripplanner.core.model.id.FeedScopedId;
 import org.opentripplanner.model.fare.FareProduct;
+import org.opentripplanner.utils.lang.IntUtils;
 import org.opentripplanner.utils.tostring.ToStringBuilder;
 
 public final class FareTransferRule implements Serializable {
@@ -33,7 +34,11 @@ public final class FareTransferRule implements Serializable {
     this.fareProducts = List.copyOf(b.fareProducts());
     this.fromLegGroup = b.fromLegGroup();
     this.toLegGroup = b.toLegGroup();
-    this.transferCount = b.transferCount();
+    this.transferCount = IntUtils.requireInRange(
+      b.transferCount(),
+      UNLIMITED_TRANSFERS,
+      Integer.MAX_VALUE
+    );
     this.timeLimit = b.timeLimit();
   }
 
@@ -49,7 +54,7 @@ public boolean containsWildCard() {
   }
 
   /**
-   * Returns true if there is no limit on the number of transfers.
+   * Returns true if there is no limit on the number of transfers or if limit unknown.
    */
   public boolean unlimitedTransfers() {
     return transferCount == UNLIMITED_TRANSFERS;

application/src/main/java/org/opentripplanner/gtfs/mapping/FareTransferRuleMapper.java

@@ -12,8 +12,6 @@
 
 class FareTransferRuleMapper {
 
-  public final int MISSING_VALUE = -999;
-
   private final IdFactory idFactory;
   private final FareProductMapper fareProductMapper;
 
@@ -36,9 +34,11 @@ private FareTransferRule doMap(org.onebusaway.gtfs.model.FareTransferRule rhs) {
       .withId(idFactory.createId(rhs.getId(), "fare transfer rule"))
       .withFromLegGroup(idFactory.createNullableId(rhs.getFromLegGroupId()))
       .withToLegGroup(idFactory.createNullableId(rhs.getToLegGroupId()))
-      .withTransferCount(rhs.getTransferCount())
       .withFareProducts(products);
-    if (rhs.getDurationLimit() != MISSING_VALUE) {
+    if (rhs.isTransferCountSet()) {
+      builder.withTransferCount(rhs.getTransferCount());
+    }
+    if (rhs.isDurationLimitSet()) {
       var duration = Duration.ofSeconds(rhs.getDurationLimit());
       var type = mapLimitType(rhs.getDurationLimitType());
       builder.withTimeLimit(type, duration);

application/src/test/java/org/opentripplanner/gtfs/mapping/FareTransferRuleMapperTest.java

@@ -99,6 +99,48 @@ void ruleWithoutProductIsFree() {
     assertTrue(transferRule.isFree());
   }
 
+  @Test
+  void defaultTransferCount() {
+    var rule = new FareTransferRule();
+    rule.setFromLegGroupId(groupId1);
+    rule.setToLegGroupId(groupId2);
+
+    var transferRule = map(fareProduct(), rule);
+    assertTrue(transferRule.unlimitedTransfers());
+  }
+
+  @Test
+  void explicitUnlimitedTransfer() {
+    var rule = new FareTransferRule();
+    rule.setFromLegGroupId(groupId1);
+    rule.setToLegGroupId(groupId2);
+    rule.setTransferCount(-1);
+
+    var transferRule = map(fareProduct(), rule);
+    assertTrue(transferRule.unlimitedTransfers());
+  }
+
+  @Test
+  void transferCount() {
+    var rule = new FareTransferRule();
+    rule.setFromLegGroupId(groupId1);
+    rule.setToLegGroupId(groupId2);
+    rule.setTransferCount(2);
+
+    var transferRule = map(fareProduct(), rule);
+    assertFalse(transferRule.unlimitedTransfers());
+  }
+
+  @Test
+  void invalidTransferCount() {
+    var rule = new FareTransferRule();
+    rule.setFromLegGroupId(groupId1);
+    rule.setToLegGroupId(groupId2);
+    rule.setTransferCount(-2);
+
+    assertThrows(IllegalArgumentException.class, () -> map(fareProduct(), rule));
+  }
+
   private FareProduct fareProduct() {
     var fareProduct = new FareProduct();
     fareProduct.setId(id);