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

Open
wants to merge 40 commits into
base: dev-2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 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
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> code() {
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;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Btw can you rename this class to follow the class naming convention, idk why this is the only file that doesn't follow it. Hopefully the renaming doesn't mess up git history.

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> code();

public DataFetcher<String> entranceId();

public DataFetcher<String> name();

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 @@ -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
Expand Up @@ -65,7 +65,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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,19 @@ class VertexGenerator {
private final HashMap<Long, Map<OsmLevel, OsmVertex>> multiLevelNodes = new HashMap<>();
private final OsmDatabase osmdb;
private final Set<String> boardingAreaRefTags;
private final Boolean includeOsmSubwayEntrances;
private final VertexFactory vertexFactory;

public VertexGenerator(OsmDatabase osmdb, Graph graph, Set<String> boardingAreaRefTags) {
public VertexGenerator(
OsmDatabase osmdb,
Graph graph,
Set<String> boardingAreaRefTags,
boolean includeOsmSubwayEntrances
) {
this.osmdb = osmdb;
this.vertexFactory = new VertexFactory(graph);
this.boardingAreaRefTags = boardingAreaRefTags;
this.includeOsmSubwayEntrances = includeOsmSubwayEntrances;
}

/**
Expand Down Expand Up @@ -95,6 +102,13 @@ IntersectionVertex getVertexForOsmNode(OsmNode node, OsmWithTags way) {
iv = bv;
}

if (includeOsmSubwayEntrances && node.isSubwayEntrance()) {
String ref = node.getTag("ref");

boolean accessible = node.isTag("wheelchair", "yes");
Copy link
Member

Choose a reason for hiding this comment

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

This has three states instead of two, use the unknown one when the tag isn't specified.

iv = vertexFactory.stationEntrance(nid, coordinate, ref, accessible);
}

if (iv == null) {
iv =
vertexFactory.osm(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,28 @@
* Example: {@code "osm" : [ {source: "file:///path/to/otp/norway.pbf"} ] }
*
*/
public record OsmExtractParameters(URI source, OsmTagMapperSource osmTagMapper, ZoneId timeZone)
public record OsmExtractParameters(
URI source,
OsmTagMapperSource osmTagMapper,
ZoneId timeZone,
boolean includeOsmSubwayEntrances
)
implements DataSourceConfig {
public static final OsmTagMapperSource DEFAULT_OSM_TAG_MAPPER = OsmTagMapperSource.DEFAULT;

public static final ZoneId DEFAULT_TIME_ZONE = null;

public static final boolean DEFAULT_INCLUDE_OSM_SUBWAY_ENTRANCES = false;

public static final OsmExtractParameters DEFAULT = new OsmExtractParametersBuilder().build();

OsmExtractParameters(OsmExtractParametersBuilder builder) {
this(builder.getSource(), builder.getOsmTagMapper(), builder.getTimeZone());
this(
builder.getSource(),
builder.getOsmTagMapper(),
builder.getTimeZone(),
builder.includeOsmSubwayEntrances()
);
}

@Override
Expand All @@ -37,6 +49,10 @@ public ZoneId timeZone() {
return timeZone;
}

public boolean includeOsmSubwayEntrances() {
return includeOsmSubwayEntrances;
}

public OsmExtractParametersBuilder copyOf() {
return new OsmExtractParametersBuilder(this);
}
Expand Down
Loading
Loading