Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If configured, add subway station entrances from OSM to walk steps #6343

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
1d19a05
Add subway station entrances to walk steps
HenrikSundell Sep 18, 2024
67f4b1b
Add entity to walk steps
HenrikSundell Oct 2, 2024
9d18269
Add more parameters to Entrance
HenrikSundell Oct 3, 2024
3b88288
Move StepEntity classes
HenrikSundell Oct 7, 2024
0a62bf8
Remove default name for subway station entrances
HenrikSundell Oct 16, 2024
528ab55
Add option to turn on osm subway entrances in osmDefaults
HenrikSundell Oct 18, 2024
3e4ae96
Merge remote-tracking branch 'otp/dev-2.x' into station-entrances
HenrikSundell Oct 19, 2024
401a405
Fix walk step generation
HenrikSundell Oct 21, 2024
438bc31
Add step entity to graphql tests
HenrikSundell Oct 23, 2024
97c2de6
Rename variables to match with graphql
HenrikSundell Oct 25, 2024
d844561
Rename variables
HenrikSundell Oct 25, 2024
02a07ea
Rename function
HenrikSundell Oct 27, 2024
5dc74dd
Fix comments
HenrikSundell Oct 28, 2024
5d55646
Remove println
HenrikSundell Oct 28, 2024
d1067c6
Remove unnecessary imports
HenrikSundell Oct 28, 2024
5c97ad1
Add accessibilty information to entrances
HenrikSundell Oct 28, 2024
e10e0a2
Use existing entrance class for walk steps
HenrikSundell Nov 7, 2024
34761fe
Fix EntranceImpl
HenrikSundell Nov 7, 2024
c84b7cf
Add id to walk step entrances
HenrikSundell Nov 8, 2024
2ea8a52
Remove old file
HenrikSundell Nov 8, 2024
39b0db3
Fix otp version
HenrikSundell Nov 8, 2024
3b6bf3f
Remove unused import
HenrikSundell Nov 8, 2024
36be3dc
Merge remote-tracking branch 'otp/dev-2.x' into station-entrances
HenrikSundell Nov 8, 2024
c9df52d
Require entranceId
HenrikSundell Nov 10, 2024
7a9a8f6
Rename methods
HenrikSundell Nov 10, 2024
c9139e3
Update dosumentation
HenrikSundell Nov 11, 2024
7b21024
Update documentation
HenrikSundell Nov 11, 2024
ce7719c
Remove redundant null check
HenrikSundell Nov 11, 2024
b737411
Add feature union to steps
HenrikSundell Nov 14, 2024
f547e07
Return feature based on relativeDirection
HenrikSundell Nov 14, 2024
18b84f0
Remove StepFeature class
HenrikSundell Nov 14, 2024
a327594
Merge remote-tracking branch 'upstream/dev-2.x' into station-entrances
leonardehrenfried Dec 17, 2024
04d35b7
Clean up code a little
leonardehrenfried Dec 17, 2024
c4d665d
Reformat code and schema
leonardehrenfried Dec 17, 2024
bf89f49
Fix tests
leonardehrenfried Dec 17, 2024
2eb0e7b
Add documentation
leonardehrenfried Dec 18, 2024
4fd0f68
Merge remote-tracking branch 'upstream/dev-2.x' into station-entrances
leonardehrenfried Dec 18, 2024
977d8eb
Clean up
leonardehrenfried Dec 18, 2024
b7cc6fd
Remove enum mapper test for REST API
leonardehrenfried Dec 18, 2024
e473061
Fix highway exits
leonardehrenfried Dec 18, 2024
713143b
Use three states for accessibility
leonardehrenfried Jan 2, 2025
0b6a74a
Update documentation
leonardehrenfried Jan 2, 2025
5b12c7f
Update docs
leonardehrenfried Jan 6, 2025
8c6fda4
Extract entrance from transit link
leonardehrenfried Jan 6, 2025
bd94b13
Add test for extracting entrance from pathway data
leonardehrenfried Jan 6, 2025
74106b5
Update schema docs
leonardehrenfried Jan 6, 2025
628bf95
Rename 'code' to 'publicCode'
leonardehrenfried Jan 7, 2025
4248ffe
Fix mapping in Transmodel API
leonardehrenfried Jan 10, 2025
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 @@ -8,39 +8,11 @@
import java.util.Map;
import java.util.function.Function;
import org.junit.jupiter.api.Test;
import org.opentripplanner.ext.restapi.model.ApiAbsoluteDirection;
import org.opentripplanner.ext.restapi.model.ApiRelativeDirection;
import org.opentripplanner.ext.restapi.model.ApiVertexType;
import org.opentripplanner.model.plan.AbsoluteDirection;
import org.opentripplanner.model.plan.RelativeDirection;
import org.opentripplanner.model.plan.VertexType;

public class EnumMapperTest {

private static final String MSG =
"Assert that the API enums have the exact same values that " +
"the domain enums of the same type, and that the specialized mapper is mapping all " +
"values. If this assumtion does not hold, create a new test.";

@Test
public void map() {
try {
verifyExactMatch(
AbsoluteDirection.class,
ApiAbsoluteDirection.class,
AbsoluteDirectionMapper::mapAbsoluteDirection
);
verifyExactMatch(
RelativeDirection.class,
ApiRelativeDirection.class,
RelativeDirectionMapper::mapRelativeDirection
);
} catch (RuntimeException ex) {
System.out.println(MSG);
throw ex;
}
}

@Test
public void testVertexTypeMapping() {
verifyExplicitMatch(
Expand Down Expand Up @@ -75,17 +47,4 @@ private <D extends Enum<?>, A extends Enum<?>> void verifyExplicitMatch(
assertTrue(rest.isEmpty());
}

private <D extends Enum<?>, A extends Enum<?>> void verifyExactMatch(
Class<D> domainClass,
Class<A> apiClass,
Function<D, A> mapper
) {
List<A> rest = new ArrayList<>(List.of(apiClass.getEnumConstants()));
for (D it : domainClass.getEnumConstants()) {
A result = mapper.apply(it);
assertEquals(result.name(), it.name());
rest.remove(result);
}
assertTrue(rest.isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public static ApiRelativeDirection mapRelativeDirection(RelativeDirection domain
case HARD_LEFT -> ApiRelativeDirection.HARD_LEFT;
case LEFT -> ApiRelativeDirection.LEFT;
case SLIGHTLY_LEFT -> ApiRelativeDirection.SLIGHTLY_LEFT;
case CONTINUE -> ApiRelativeDirection.CONTINUE;
case CONTINUE, ENTER_OR_EXIT_STATION -> ApiRelativeDirection.CONTINUE;
case SLIGHTLY_RIGHT -> ApiRelativeDirection.SLIGHTLY_RIGHT;
case RIGHT -> ApiRelativeDirection.RIGHT;
case HARD_RIGHT -> ApiRelativeDirection.HARD_RIGHT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public ApiWalkStep mapWalkStep(WalkStep domain) {
api.streetName = domain.getDirectionText().toString(locale);
api.absoluteDirection =
domain.getAbsoluteDirection().map(AbsoluteDirectionMapper::mapAbsoluteDirection).orElse(null);
api.exit = domain.getExit();
api.exit = domain.highwayExit().orElse(null);
api.stayOn = domain.isStayOn();
api.area = domain.getArea();
api.bogusName = domain.nameIsDerived();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opentripplanner.apis.gtfs.datafetchers.CurrencyImpl;
import org.opentripplanner.apis.gtfs.datafetchers.DefaultFareProductImpl;
import org.opentripplanner.apis.gtfs.datafetchers.DepartureRowImpl;
import org.opentripplanner.apis.gtfs.datafetchers.EntranceImpl;
import org.opentripplanner.apis.gtfs.datafetchers.EstimatedTimeImpl;
import org.opentripplanner.apis.gtfs.datafetchers.FareProductTypeResolver;
import org.opentripplanner.apis.gtfs.datafetchers.FareProductUseImpl;
Expand All @@ -64,6 +65,7 @@
import org.opentripplanner.apis.gtfs.datafetchers.RouteImpl;
import org.opentripplanner.apis.gtfs.datafetchers.RouteTypeImpl;
import org.opentripplanner.apis.gtfs.datafetchers.RoutingErrorImpl;
import org.opentripplanner.apis.gtfs.datafetchers.StepFeatureTypeResolver;
import org.opentripplanner.apis.gtfs.datafetchers.StopCallImpl;
import org.opentripplanner.apis.gtfs.datafetchers.StopGeometriesImpl;
import org.opentripplanner.apis.gtfs.datafetchers.StopImpl;
Expand Down Expand Up @@ -137,6 +139,7 @@ protected static GraphQLSchema buildSchema() {
.type("AlertEntity", type -> type.typeResolver(new AlertEntityTypeResolver()))
.type("CallStopLocation", type -> type.typeResolver(new CallStopLocationTypeResolver()))
.type("CallScheduledTime", type -> type.typeResolver(new CallScheduledTimeTypeResolver()))
.type("StepFeature", type -> type.typeResolver(new StepFeatureTypeResolver()))
.type(typeWiring.build(AgencyImpl.class))
.type(typeWiring.build(AlertImpl.class))
.type(typeWiring.build(BikeParkImpl.class))
Expand Down Expand Up @@ -195,6 +198,7 @@ protected static GraphQLSchema buildSchema() {
.type(typeWiring.build(LegTimeImpl.class))
.type(typeWiring.build(RealTimeEstimateImpl.class))
.type(typeWiring.build(EstimatedTimeImpl.class))
.type(typeWiring.build(EntranceImpl.class))
.build();
SchemaGenerator schemaGenerator = new SchemaGenerator();
return schemaGenerator.makeExecutableSchema(typeRegistry, runtimeWiring);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.opentripplanner.apis.gtfs.datafetchers;

import graphql.schema.DataFetcher;
import org.opentripplanner.apis.gtfs.GraphQLUtils;
import org.opentripplanner.apis.gtfs.generated.GraphQLDataFetchers;
import org.opentripplanner.apis.gtfs.generated.GraphQLTypes;
import org.opentripplanner.transit.model.site.Entrance;

public class EntranceImpl implements GraphQLDataFetchers.GraphQLEntrance {

@Override
public DataFetcher<String> publicCode() {
return environment -> {
Entrance entrance = environment.getSource();
return entrance.getCode();
};
}

@Override
public DataFetcher<String> entranceId() {
return environment -> {
Entrance entrance = environment.getSource();
return entrance.getId().toString();
};
}

@Override
public DataFetcher<String> name() {
return environment -> {
Entrance entrance = environment.getSource();
return org.opentripplanner.framework.graphql.GraphQLUtils.getTranslation(
entrance.getName(),
environment
);
};
}

@Override
public DataFetcher<GraphQLTypes.GraphQLWheelchairBoarding> wheelchairAccessible() {
return environment -> {
Entrance entrance = environment.getSource();
return GraphQLUtils.toGraphQL(entrance.getWheelchairAccessibility());
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.opentripplanner.apis.gtfs.datafetchers;

import graphql.TypeResolutionEnvironment;
import graphql.schema.GraphQLObjectType;
import graphql.schema.GraphQLSchema;
import graphql.schema.TypeResolver;
import org.opentripplanner.transit.model.site.Entrance;

public class StepFeatureTypeResolver implements TypeResolver {

@Override
public GraphQLObjectType getType(TypeResolutionEnvironment environment) {
Object o = environment.getObject();
GraphQLSchema schema = environment.getSchema();

if (o instanceof Entrance) {
return schema.getObjectType("Entrance");
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ public DataFetcher<Iterable<Step>> elevationProfile() {

@Override
public DataFetcher<String> exit() {
return environment -> getSource(environment).getExit();
return environment -> getSource(environment).highwayExit().orElse(null);
}

@Override
public DataFetcher<Object> feature() {
return environment -> getSource(environment).entrance().orElse(null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,17 @@ public interface GraphQLEmissions {
public DataFetcher<org.opentripplanner.framework.model.Grams> co2();
}

/** Station entrance or exit, originating from OSM or GTFS data. */
public interface GraphQLEntrance {
public DataFetcher<String> entranceId();

public DataFetcher<String> name();

public DataFetcher<String> publicCode();

public DataFetcher<org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLWheelchairBoarding> wheelchairAccessible();
}

/** Real-time estimates for an arrival or departure at a certain place. */
public interface GraphQLEstimatedTime {
public DataFetcher<java.time.Duration> delay();
Expand Down Expand Up @@ -1024,6 +1035,9 @@ public interface GraphQLRoutingError {
public DataFetcher<GraphQLInputField> inputField();
}

/** A feature for a step */
public interface GraphQLStepFeature extends TypeResolver {}

/**
* Stop can represent either a single public transport stop, where passengers can
* board and/or disembark vehicles, or a station, which contains multiple stops.
Expand Down Expand Up @@ -1521,6 +1535,8 @@ public interface GraphQLStep {

public DataFetcher<String> exit();

public DataFetcher<Object> feature();

public DataFetcher<Double> lat();

public DataFetcher<Double> lon();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4327,7 +4327,11 @@ public enum GraphQLRealtimeState {
UPDATED,
}

/** Actions to take relative to the current position when engaging a walking/driving step. */
/**
* A direction that is not absolute but rather fuzzy and context-dependent.
* It provides the passenger with information what they should do in this step depending on where they
* were in the previous one.
*/
public enum GraphQLRelativeDirection {
CIRCLE_CLOCKWISE,
CIRCLE_COUNTERCLOCKWISE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static GraphQLRelativeDirection map(RelativeDirection relativeDirection)
case HARD_LEFT -> GraphQLRelativeDirection.HARD_LEFT;
case LEFT -> GraphQLRelativeDirection.LEFT;
case SLIGHTLY_LEFT -> GraphQLRelativeDirection.SLIGHTLY_LEFT;
case CONTINUE -> GraphQLRelativeDirection.CONTINUE;
case CONTINUE, ENTER_OR_EXIT_STATION -> GraphQLRelativeDirection.CONTINUE;
case SLIGHTLY_RIGHT -> GraphQLRelativeDirection.SLIGHTLY_RIGHT;
case RIGHT -> GraphQLRelativeDirection.RIGHT;
case HARD_RIGHT -> GraphQLRelativeDirection.HARD_RIGHT;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.opentripplanner.apis.transmodel.mapping;

import org.opentripplanner.model.plan.RelativeDirection;

/**
* This mapper makes sure that only those values are returned which have a mapping in the Transmodel API,
* as we don't really want to return all of them.
*/
public class RelativeDirectionMapper {

public static RelativeDirection map(RelativeDirection relativeDirection) {
return switch (relativeDirection) {
case DEPART,
SLIGHTLY_LEFT,
HARD_LEFT,
LEFT,
CONTINUE,
SLIGHTLY_RIGHT,
RIGHT,
HARD_RIGHT,
CIRCLE_CLOCKWISE,
CIRCLE_COUNTERCLOCKWISE,
ELEVATOR,
UTURN_LEFT,
UTURN_RIGHT -> relativeDirection;
// for these the Transmodel API doesn't have a mapping. should it?
case ENTER_STATION,
EXIT_STATION,
ENTER_OR_EXIT_STATION,
FOLLOW_SIGNS -> RelativeDirection.CONTINUE;
Comment on lines +27 to +30
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late response here, but this can be dropped. We will just add the new enum values to the Transmodel GraphQL API - I can do it if you want.

The JS GraphQL lib has a validator to compare to schema files for breaking changes - adding Enum values are considered a dangerous change - not a braking change. It should be handled by defensive programming on the client side. So, at entur we want to add the new enum values and we will cross fingers and hope our clients did the right thing.

Copy link
Member

Choose a reason for hiding this comment

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

I also want to add a better unit-test to the enums we have today - so I can do that in the same change.

Copy link
Member

Choose a reason for hiding this comment

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

Could/should we create a CI test for validating that we are not creating breaking schema changes with the JS GraphQL lib?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we still need the mapper so to we don't ever expose ENTER_OR_EXIT_STATION.

};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import graphql.schema.GraphQLList;
import graphql.schema.GraphQLNonNull;
import graphql.schema.GraphQLObjectType;
import org.opentripplanner.apis.transmodel.mapping.RelativeDirectionMapper;
import org.opentripplanner.apis.transmodel.model.EnumTypes;
import org.opentripplanner.framework.graphql.GraphQLUtils;
import org.opentripplanner.model.plan.WalkStep;
Expand All @@ -31,7 +32,9 @@ public static GraphQLObjectType create(GraphQLObjectType elevationStepType) {
.name("relativeDirection")
.description("The relative direction of this step.")
.type(EnumTypes.RELATIVE_DIRECTION)
.dataFetcher(environment -> ((WalkStep) environment.getSource()).getRelativeDirection())
.dataFetcher(environment ->
RelativeDirectionMapper.map(((WalkStep) environment.getSource()).getRelativeDirection())
)
.build()
)
.field(
Expand Down Expand Up @@ -65,7 +68,9 @@ public static GraphQLObjectType create(GraphQLObjectType elevationStepType) {
.name("exit")
.description("When exiting a highway or traffic circle, the exit name/number.")
.type(Scalars.GraphQLString)
.dataFetcher(environment -> ((WalkStep) environment.getSource()).getExit())
.dataFetcher(environment ->
((WalkStep) environment.getSource()).highwayExit().orElse(null)
)
.build()
)
.field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ static OsmModule provideOsmModule(
osmConfiguredDataSource.dataSource(),
osmConfiguredDataSource.config().osmTagMapper(),
osmConfiguredDataSource.config().timeZone(),
osmConfiguredDataSource.config().includeOsmSubwayEntrances(),
config.osmCacheDataInMem,
issueStore
)
Expand All @@ -86,6 +87,7 @@ static OsmModule provideOsmModule(
.withStaticBikeParkAndRide(config.staticBikeParkAndRide)
.withMaxAreaNodes(config.maxAreaNodes)
.withBoardingAreaRefTags(config.boardingLocationTags)
.withIncludeOsmSubwayEntrances(config.osmDefaults.includeOsmSubwayEntrances())
.withIssueStore(issueStore)
.withStreetLimitationParameters(streetLimitationParameters)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void buildElevatorEdges(Graph graph) {
}
int travelTime = parseDuration(node).orElse(-1);

var wheelchair = node.getWheelchairAccessibility();
var wheelchair = node.wheelchairAccessibility();

createElevatorHopEdges(
onboardVertices,
Expand Down Expand Up @@ -138,7 +138,7 @@ public void buildElevatorEdges(Graph graph) {

int travelTime = parseDuration(elevatorWay).orElse(-1);
int levels = nodes.size();
var wheelchair = elevatorWay.getWheelchairAccessibility();
var wheelchair = elevatorWay.wheelchairAccessibility();

createElevatorHopEdges(
onboardVertices,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ public class OsmModule implements GraphBuilderModule {
this.issueStore = issueStore;
this.params = params;
this.osmdb = new OsmDatabase(issueStore);
this.vertexGenerator = new VertexGenerator(osmdb, graph, params.boardingAreaRefTags());
this.vertexGenerator =
new VertexGenerator(
osmdb,
graph,
params.boardingAreaRefTags(),
params.includeOsmSubwayEntrances()
);
this.normalizer = new SafetyValueNormalizer(graph, issueStore);
this.streetLimitationParameters = Objects.requireNonNull(streetLimitationParameters);
this.parkingRepository = parkingService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class OsmModuleBuilder {
private boolean platformEntriesLinking = false;
private boolean staticParkAndRide = false;
private boolean staticBikeParkAndRide = false;
private boolean includeOsmSubwayEntrances = false;
private int maxAreaNodes;
private StreetLimitationParameters streetLimitationParameters = new StreetLimitationParameters();

Expand Down Expand Up @@ -79,6 +80,11 @@ public OsmModuleBuilder withMaxAreaNodes(int maxAreaNodes) {
return this;
}

public OsmModuleBuilder withIncludeOsmSubwayEntrances(boolean includeOsmSubwayEntrances) {
this.includeOsmSubwayEntrances = includeOsmSubwayEntrances;
return this;
}

public OsmModuleBuilder withStreetLimitationParameters(StreetLimitationParameters parameters) {
this.streetLimitationParameters = parameters;
return this;
Expand All @@ -98,7 +104,8 @@ public OsmModule build() {
areaVisibility,
platformEntriesLinking,
staticParkAndRide,
staticBikeParkAndRide
staticBikeParkAndRide,
includeOsmSubwayEntrances
)
);
}
Expand Down
Loading
Loading