Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.List;
import org.junit.jupiter.api.Test;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;

class SmooveBikeRentalDataSourceTest {
Expand All @@ -18,7 +19,8 @@ void makeStation() {
"file:src/ext-test/resources/smoovebikerental/smoove.json",
null,
true,
HttpHeaders.empty()
HttpHeaders.empty(),
AllowedRentalType.ALL
)
);
assertTrue(source.update());
Expand Down Expand Up @@ -84,7 +86,8 @@ void makeStationWithoutOverloading() {
"file:src/ext-test/resources/smoovebikerental/smoove.json",
null,
false,
HttpHeaders.empty()
HttpHeaders.empty(),
AllowedRentalType.ALL
)
);
assertTrue(source.update());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.ext.smoovebikerental;

import javax.annotation.Nullable;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;
import org.opentripplanner.updater.vehicle_rental.datasources.params.VehicleRentalDataSourceParameters;
Expand All @@ -12,7 +13,8 @@ public record SmooveBikeRentalDataSourceParameters(
String url,
String network,
boolean overloadingAllowed,
HttpHeaders httpHeaders
HttpHeaders httpHeaders,
AllowedRentalType allowedRentalType
)
implements VehicleRentalDataSourceParameters {
/**
Expand All @@ -29,4 +31,9 @@ public String getNetwork(String defaultValue) {
public VehicleRentalSourceType sourceType() {
return VehicleRentalSourceType.SMOOVE;
}

@Override
public AllowedRentalType allowedRentalType() {
return allowedRentalType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ private static List<GbfsVehicleRentalDataSourceParameters> buildListOfNetworksFr
networkName,
networkParams.geofencingZones(),
// overloadingAllowed - not part of GBFS, not supported here
false
false,
// allowedRentalType not supported
null
)
);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_1;
import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_2;
import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_3;
import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_7;

import org.opentripplanner.ext.smoovebikerental.SmooveBikeRentalDataSourceParameters;
import org.opentripplanner.standalone.config.framework.json.NodeAdapter;
import org.opentripplanner.standalone.config.routerconfig.updaters.HttpHeadersConfig;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;
import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters;
Expand Down Expand Up @@ -43,13 +45,15 @@ public VehicleRentalDataSourceParameters create() {
headers(),
network(),
geofencingZones(),
overloadingAllowed()
overloadingAllowed(),
allowedRentalType()
);
case SMOOVE -> new SmooveBikeRentalDataSourceParameters(
url(),
network(),
overloadingAllowed(),
headers()
headers(),
allowedRentalType()
);
};
}
Expand Down Expand Up @@ -121,4 +125,21 @@ private boolean geofencingZones() {
)
.asBoolean(false);
}

private AllowedRentalType allowedRentalType() {
return c
.of("allowedRentalType")
.since(V2_7)
.summary("The type of rental data to include.")
.description(
"""
The type of rental data to include. This can be one of the following:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarify that this parameter is only temporary and it's going to be removed.


- `ALL`: Include all data types.
- `STATIONS`: Include station data only.
- `VEHICLES`: Include floating vehicle data only.
"""
)
.asEnum(AllowedRentalType.ALL);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.opentripplanner.updater;

public enum AllowedRentalType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class should be inside org.opentripplanner.updater.vehicle_rental.datasources.params package.

STATIONS,
VEHICLES,
ALL,
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.opentripplanner.service.vehiclerental.model.RentalVehicleType;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalSystem;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -76,60 +77,72 @@ public List<VehicleRentalPlace> getUpdates() {

List<VehicleRentalPlace> stations = new LinkedList<>();

// Both station information and status are required for all systems using stations
GBFSStationInformation stationInformation = loader.getFeed(GBFSStationInformation.class);
GBFSStationStatus stationStatus = loader.getFeed(GBFSStationStatus.class);
if (stationInformation != null && stationStatus != null) {
// Index all the station status entries on their station ID.
Map<String, GBFSStation> statusLookup = stationStatus
.getData()
.getStations()
.stream()
.collect(Collectors.toMap(GBFSStation::getStationId, Function.identity()));
GbfsStationStatusMapper stationStatusMapper = new GbfsStationStatusMapper(
statusLookup,
vehicleTypes
);
GbfsStationInformationMapper stationInformationMapper = new GbfsStationInformationMapper(
system,
vehicleTypes,
params.allowKeepingRentedVehicleAtDestination(),
params.overloadingAllowed()
);

// Iterate over all known stations, and if we have any status information add it to those station objects.
stations.addAll(
stationInformation
if (
params.allowedRentalType() == null ||
params.allowedRentalType() == AllowedRentalType.ALL ||
params.allowedRentalType() == AllowedRentalType.STATIONS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could extract this code to be a method inside the params.

) {
// Both station information and status are required for all systems using stations
GBFSStationInformation stationInformation = loader.getFeed(GBFSStationInformation.class);
GBFSStationStatus stationStatus = loader.getFeed(GBFSStationStatus.class);
if (stationInformation != null && stationStatus != null) {
// Index all the station status entries on their station ID.
Map<String, GBFSStation> statusLookup = stationStatus
.getData()
.getStations()
.stream()
.map(stationInformationMapper::mapStationInformation)
.filter(Objects::nonNull)
.peek(stationStatusMapper::fillStationStatus)
.toList()
);
}

// Append the floating bike stations.
if (OTPFeature.FloatingBike.isOn()) {
GBFSFreeBikeStatus freeBikeStatus = loader.getFeed(GBFSFreeBikeStatus.class);
if (freeBikeStatus != null) {
GbfsFreeVehicleStatusMapper freeVehicleStatusMapper = new GbfsFreeVehicleStatusMapper(
system,
.collect(Collectors.toMap(GBFSStation::getStationId, Function.identity()));
GbfsStationStatusMapper stationStatusMapper = new GbfsStationStatusMapper(
statusLookup,
vehicleTypes
);
GbfsStationInformationMapper stationInformationMapper = new GbfsStationInformationMapper(
system,
vehicleTypes,
params.allowKeepingRentedVehicleAtDestination(),
params.overloadingAllowed()
);

// Iterate over all known stations, and if we have any status information add it to those station objects.
stations.addAll(
freeBikeStatus
stationInformation
.getData()
.getBikes()
.getStations()
.stream()
.map(freeVehicleStatusMapper::mapFreeVehicleStatus)
.map(stationInformationMapper::mapStationInformation)
.filter(Objects::nonNull)
.peek(stationStatusMapper::fillStationStatus)
.toList()
);
}
}

if (
params.allowedRentalType() == null ||
params.allowedRentalType() == AllowedRentalType.ALL ||
params.allowedRentalType() == AllowedRentalType.VEHICLES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above and you could do the OTPFeature.FloatingBike.isOn() check inside the same if.

) {
// Append the floating bike stations.
if (OTPFeature.FloatingBike.isOn()) {
GBFSFreeBikeStatus freeBikeStatus = loader.getFeed(GBFSFreeBikeStatus.class);
if (freeBikeStatus != null) {
GbfsFreeVehicleStatusMapper freeVehicleStatusMapper = new GbfsFreeVehicleStatusMapper(
system,
vehicleTypes
);
stations.addAll(
freeBikeStatus
.getData()
.getBikes()
.stream()
.map(freeVehicleStatusMapper::mapFreeVehicleStatus)
.filter(Objects::nonNull)
.toList()
);
}
}
}

if (params.geofencingZones()) {
var zones = loader.getFeed(GBFSGeofencingZones.class);
if (zones != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.opentripplanner.updater.vehicle_rental.datasources.params;

import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;

Expand All @@ -10,11 +11,17 @@ public record GbfsVehicleRentalDataSourceParameters(
HttpHeaders httpHeaders,
String network,
boolean geofencingZones,
boolean overloadingAllowed
boolean overloadingAllowed,
AllowedRentalType allowedRentalType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a constructor that makes sure this is never null. Like this:

public AlertUrl {
Objects.requireNonNull(uri);
}

)
implements VehicleRentalDataSourceParameters {
@Override
public VehicleRentalSourceType sourceType() {
return VehicleRentalSourceType.GBFS;
}

@Override
public AllowedRentalType allowedRentalType() {
return allowedRentalType;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.updater.vehicle_rental.datasources.params;

import javax.annotation.Nullable;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;

Expand All @@ -13,4 +14,6 @@ public interface VehicleRentalDataSourceParameters {
VehicleRentalSourceType sourceType();

HttpHeaders httpHeaders();

AllowedRentalType allowedRentalType();
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.opentripplanner.service.vehiclerental.internal.DefaultVehicleRentalService;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.transit.service.TimetableRepository;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.DefaultRealTimeUpdateContext;
import org.opentripplanner.updater.GraphUpdaterManager;
import org.opentripplanner.updater.GraphWriterRunnable;
Expand Down Expand Up @@ -103,5 +104,10 @@ public VehicleRentalSourceType sourceType() {
public HttpHeaders httpHeaders() {
return HttpHeaders.empty();
}

@Override
public AllowedRentalType allowedRentalType() {
return AllowedRentalType.ALL;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.opentripplanner.service.vehiclerental.model.GeofencingZone;
import org.opentripplanner.service.vehiclerental.model.RentalVehicleType;
import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
import org.opentripplanner.updater.AllowedRentalType;
import org.opentripplanner.updater.spi.HttpHeaders;
import org.opentripplanner.updater.vehicle_rental.datasources.params.GbfsVehicleRentalDataSourceParameters;

Expand All @@ -32,7 +33,8 @@ void makeStationFromV22() {
HttpHeaders.empty(),
null,
false,
false
false,
AllowedRentalType.ALL
),
new OtpHttpClientFactory()
);
Expand Down Expand Up @@ -123,7 +125,8 @@ void geofencing() {
HttpHeaders.empty(),
null,
true,
false
false,
AllowedRentalType.ALL
),
new OtpHttpClientFactory()
);
Expand Down Expand Up @@ -165,7 +168,8 @@ void makeStationFromV10() {
HttpHeaders.empty(),
network,
false,
true
true,
AllowedRentalType.ALL
),
new OtpHttpClientFactory()
);
Expand Down
16 changes: 16 additions & 0 deletions doc/user/UpdaterConfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ GBFS form factors:
|---------------------------------------------------------------------------------------|:---------------:|---------------------------------------------------------------------------------|:----------:|---------------|:-----:|
| type = "vehicle-rental" | `enum` | The type of the updater. | *Required* | | 1.5 |
| [allowKeepingRentedVehicleAtDestination](#u_1_allowKeepingRentedVehicleAtDestination) | `boolean` | If a vehicle should be allowed to be kept at the end of a station-based rental. | *Optional* | `false` | 2.1 |
| [allowedRentalType](#u_1_allowedRentalType) | `enum` | The type of rental data to include. | *Optional* | `"all"` | 2.7 |
| frequency | `duration` | How often the data should be updated. | *Optional* | `"PT1M"` | 1.5 |
| [geofencingZones](#u_1_geofencingZones) | `boolean` | Compute rental restrictions based on GBFS 2.2 geofencing zones. | *Optional* | `false` | 2.3 |
| language | `string` | TODO | *Optional* | | 2.1 |
Expand Down Expand Up @@ -346,6 +347,21 @@ For this to be possible three things need to be configured:
- If keeping the vehicle at the destination should be discouraged, then `keepingRentedVehicleAtDestinationCost` (default: 0) may also be set in the routing defaults.


<h4 id="u_1_allowedRentalType">allowedRentalType</h4>

**Since version:** `2.7` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"all"`
**Path:** /updaters/[1]
**Enum values:** `stations` | `vehicles` | `all`

The type of rental data to include.

The type of rental data to include. This can be one of the following:

- `ALL`: Include all data types.
- `STATIONS`: Include station data only.
- `VEHICLES`: Include floating vehicle data only.


<h4 id="u_1_geofencingZones">geofencingZones</h4>

**Since version:** `2.3` ∙ **Type:** `boolean` ∙ **Cardinality:** `Optional` ∙ **Default value:** `false`
Expand Down
Loading