Merge pull request #5329 from entur/fix_vehicle_filtering_in_bike_ren…

…tal_station_api

Fix  filtering of rental vehicle in vehicle station BBox search

src/main/java/org/opentripplanner/service/vehiclerental/VehicleRentalService.java

@@ -9,7 +9,7 @@
 
 /**
  * The read-only service for getting information about rental vehicles.
- *
+ * <p>
  * For writing data see {@link VehicleRentalRepository}
  */
 public interface VehicleRentalService {
@@ -32,7 +32,7 @@ public interface VehicleRentalService {
    * over a set, but we could use a spatial index if the number of vehicle rental stations is high
    * enough for performance to be a concern.
    */
-  List<VehicleRentalPlace> getVehicleRentalStationForEnvelope(
+  List<VehicleRentalStation> getVehicleRentalStationForEnvelope(
     double minLon,
     double minLat,
     double maxLon,

src/main/java/org/opentripplanner/service/vehiclerental/internal/DefaultVehicleRentalService.java

@@ -6,6 +6,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Stream;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Envelope;
 import org.opentripplanner.service.vehiclerental.VehicleRentalRepository;
@@ -54,12 +55,7 @@ public VehicleRentalVehicle getVehicleRentalVehicle(FeedScopedId id) {
 
   @Override
   public List<VehicleRentalStation> getVehicleRentalStations() {
-    return rentalPlaces
-      .values()
-      .stream()
-      .filter(VehicleRentalStation.class::isInstance)
-      .map(VehicleRentalStation.class::cast)
-      .toList();
+    return getVehicleRentalStationsAsStream().toList();
   }
 
   @Override
@@ -100,7 +96,7 @@ public boolean hasRentalBikes() {
   }
 
   @Override
-  public List<VehicleRentalPlace> getVehicleRentalStationForEnvelope(
+  public List<VehicleRentalStation> getVehicleRentalStationForEnvelope(
     double minLon,
     double minLat,
     double maxLon,
@@ -111,10 +107,16 @@ public List<VehicleRentalPlace> getVehicleRentalStationForEnvelope(
       new Coordinate(maxLon, maxLat)
     );
 
+    return getVehicleRentalStationsAsStream()
+      .filter(b -> envelope.contains(new Coordinate(b.getLongitude(), b.getLatitude())))
+      .toList();
+  }
+
+  private Stream<VehicleRentalStation> getVehicleRentalStationsAsStream() {
     return rentalPlaces
       .values()
       .stream()
-      .filter(b -> envelope.contains(new Coordinate(b.getLongitude(), b.getLatitude())))
-      .toList();
+      .filter(VehicleRentalStation.class::isInstance)
+      .map(VehicleRentalStation.class::cast);
   }
 }

src/test/java/org/opentripplanner/service/vehiclerental/internal/DefaultVehicleRentalServiceTest.java

@@ -0,0 +1,40 @@
+package org.opentripplanner.service.vehiclerental.internal;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner.service.vehiclerental.model.TestFreeFloatingRentalVehicleBuilder;
+import org.opentripplanner.service.vehiclerental.model.TestVehicleRentalStationBuilder;
+import org.opentripplanner.service.vehiclerental.model.VehicleRentalStation;
+import org.opentripplanner.service.vehiclerental.model.VehicleRentalVehicle;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
+
+class DefaultVehicleRentalServiceTest {
+
+  @Test
+  void getVehicleRentalStationForEnvelopeShouldExcludeVehicleRentalVehicle() {
+    DefaultVehicleRentalService defaultVehicleRentalService = new DefaultVehicleRentalService();
+
+    VehicleRentalStation vehicleRentalStation = new TestVehicleRentalStationBuilder()
+      .withLatitude(1)
+      .withLongitude(1)
+      .build();
+    defaultVehicleRentalService.addVehicleRentalStation(vehicleRentalStation);
+
+    VehicleRentalVehicle vehicleRentalVehicle = new TestFreeFloatingRentalVehicleBuilder()
+      .withLatitude(2)
+      .withLongitude(2)
+      .build();
+    defaultVehicleRentalService.addVehicleRentalStation(vehicleRentalVehicle);
+
+    List<VehicleRentalStation> vehicleRentalStationForEnvelope = defaultVehicleRentalService.getVehicleRentalStationForEnvelope(
+      0,
+      0,
+      10,
+      10
+    );
+    assertEquals(1, vehicleRentalStationForEnvelope.size());
+    assertEquals(vehicleRentalStation, vehicleRentalStationForEnvelope.get(0));
+  }
+}

src/test/java/org/opentripplanner/service/vehiclerental/model/TestFreeFloatingRentalVehicleBuilder.java

@@ -8,13 +8,28 @@
 public class TestFreeFloatingRentalVehicleBuilder {
 
   public static final String NETWORK_1 = "Network-1";
+  public static final double DEFAULT_LATITUDE = 47.520;
+  public static final double DEFAULT_LONGITUDE = 19.01;
+
+  private double latitude = DEFAULT_LATITUDE;
+  private double longitude = DEFAULT_LONGITUDE;
 
   private RentalVehicleType vehicleType = RentalVehicleType.getDefaultType(NETWORK_1);
 
   public static TestFreeFloatingRentalVehicleBuilder of() {
     return new TestFreeFloatingRentalVehicleBuilder();
   }
 
+  public TestFreeFloatingRentalVehicleBuilder withLatitude(double latitude) {
+    this.latitude = latitude;
+    return this;
+  }
+
+  public TestFreeFloatingRentalVehicleBuilder withLongitude(double longitude) {
+    this.longitude = longitude;
+    return this;
+  }
+
   public TestFreeFloatingRentalVehicleBuilder withVehicleScooter() {
     return buildVehicleType(RentalFormFactor.SCOOTER);
   }
@@ -45,8 +60,8 @@ public VehicleRentalVehicle build() {
     var stationName = "free-floating-" + vehicleType.formFactor.name().toLowerCase();
     vehicle.id = new FeedScopedId(NETWORK_1, stationName);
     vehicle.name = new NonLocalizedString(stationName);
-    vehicle.latitude = 47.510;
-    vehicle.longitude = 18.99;
+    vehicle.latitude = latitude;
+    vehicle.longitude = longitude;
     vehicle.vehicleType = vehicleType;
     return vehicle;
   }

src/test/java/org/opentripplanner/service/vehiclerental/model/TestVehicleRentalStationBuilder.java

@@ -9,7 +9,11 @@
 public class TestVehicleRentalStationBuilder {
 
   public static final String NETWORK_1 = "Network-1";
+  public static final double DEFAULT_LATITUDE = 47.510;
+  public static final double DEFAULT_LONGITUDE = 18.99;
 
+  private double latitude = DEFAULT_LATITUDE;
+  private double longitude = DEFAULT_LONGITUDE;
   private int vehicles = 10;
   private int spaces = 10;
   private boolean overloadingAllowed = false;
@@ -20,6 +24,16 @@ public static TestVehicleRentalStationBuilder of() {
     return new TestVehicleRentalStationBuilder();
   }
 
+  public TestVehicleRentalStationBuilder withLatitude(double latitude) {
+    this.latitude = latitude;
+    return this;
+  }
+
+  public TestVehicleRentalStationBuilder withLongitude(double longitude) {
+    this.longitude = longitude;
+    return this;
+  }
+
   public TestVehicleRentalStationBuilder withVehicles(int vehicles) {
     this.vehicles = vehicles;
     return this;
@@ -66,8 +80,8 @@ public VehicleRentalStation build() {
     var stationName = "FooStation";
     station.id = new FeedScopedId(NETWORK_1, stationName);
     station.name = new NonLocalizedString(stationName);
-    station.latitude = 47.510;
-    station.longitude = 18.99;
+    station.latitude = latitude;
+    station.longitude = longitude;
     station.vehiclesAvailable = vehicles;
     station.spacesAvailable = spaces;
     station.vehicleTypesAvailable = Map.of(vehicleType, vehicles);