Skip to content

Commit

Permalink
Merge pull request #177 from ibi-group/upstream-merge-2023-07-19
Browse files Browse the repository at this point in the history
Upstream merge 2023-07-19
  • Loading branch information
miles-grant-ibigroup authored Jul 20, 2023
2 parents 9603546 + 33db128 commit 5e36180
Show file tree
Hide file tree
Showing 35 changed files with 344 additions and 166 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/cibuild.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ jobs:
# to Maven Central open which then hang during the package phase because the Azure (Github Actions)
# NAT drops them
# https://github.com/actions/runner-images/issues/1499
# we set nodePath and npmPath to skip downloading the node binary, which frequently times out
run: |
mvn --batch-mode jacoco:prepare-agent test jacoco:report -P prettierCheck,unit-tests -Dprettier.nodePath=node
mvn --batch-mode jacoco:prepare-agent test jacoco:report -P prettierCheck,unit-tests -Dprettier.nodePath=node -Dprettier.npmPath=npm
mvn --batch-mode package -Dmaven.test.skip -P prettierSkip
- name: Send coverage data to codecov.io
Expand Down
2 changes: 2 additions & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle
- Refactor edge constructors [#5221](https://github.com/opentripplanner/OpenTripPlanner/pull/5221)
- Upgrade to Apache HTTP Client 5 [#5228](https://github.com/opentripplanner/OpenTripPlanner/pull/5228)
- Deduplicate vertex labels to save memory [#5223](https://github.com/opentripplanner/OpenTripPlanner/pull/5223)
- Fix duplicate publishing of speed test data [#5237](https://github.com/opentripplanner/OpenTripPlanner/pull/5237)
- Flex build time and memory optimization for large zones [#5233](https://github.com/opentripplanner/OpenTripPlanner/pull/5233)
[](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE)


Expand Down
2 changes: 1 addition & 1 deletion docs/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ mkdocs-material==9.1.17
mike==1.1.2
mkdocs-no-sitemap-plugin==0.0.1
# remove this after OTP 2.4.0 has been released
mkdocs-redirects==1.2.0
mkdocs-redirects==1.2.1
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
<otp.serialization.version.id>112</otp.serialization.version.id>
<!-- Lib versions - keep list sorted on property name -->
<geotools.version>29.1</geotools.version>
<google.dagger.version>2.46.1</google.dagger.version>
<google.dagger.version>2.47</google.dagger.version>
<jackson.version>2.15.2</jackson.version>
<jersey.version>3.1.2</jersey.version>
<junit.version>5.9.3</junit.version>
Expand All @@ -79,7 +79,7 @@
When running `mvn jacoco:prepare-agent test` argLine is replaced with the one activating the agent.
-->
<argLine/>
<plugin.prettier.version>0.19</plugin.prettier.version>
<plugin.prettier.version>0.20</plugin.prettier.version>
<plugin.prettier.goal>write</plugin.prettier.goal>

<junit.tags.included></junit.tags.included>
Expand Down Expand Up @@ -937,7 +937,7 @@
<dependency>
<groupId>com.graphql-java</groupId>
<artifactId>graphql-java</artifactId>
<version>20.4</version>
<version>21.0</version>
</dependency>
<dependency>
<groupId>com.graphql-java</groupId>
Expand Down
5 changes: 3 additions & 2 deletions renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"automerge": true
},
{
"description": "automatically merge test and logging dependencies",
"description": "automatically merge test, logging and build dependencies",
"matchPackageNames": [
"org.mockito:mockito-core",
"com.tngtech.archunit:archunit",
Expand All @@ -84,7 +84,8 @@
"org.codehaus.mojo:build-helper-maven-plugin",
"org.apache.maven.plugins:maven-gpg-plugin",
"org.apache.maven.plugins:maven-source-plugin",
"io.github.git-commit-id:git-commit-id-maven-plugin"
"io.github.git-commit-id:git-commit-id-maven-plugin",
"com.hubspot.maven.plugins:prettier-maven-plugin"
],
"matchPackagePrefixes": [
"org.junit.jupiter:",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package org.opentripplanner.ext.flex;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.DynamicTest.dynamicTest;
import static org.opentripplanner._support.geometry.Coordinates.BERLIN;
import static org.opentripplanner._support.geometry.Coordinates.HAMBURG;
import static org.opentripplanner.street.model.StreetTraversalPermission.ALL;
import static org.opentripplanner.street.model.StreetTraversalPermission.BICYCLE_AND_CAR;
import static org.opentripplanner.street.model.StreetTraversalPermission.CAR;
import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN;
import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN_AND_CAR;

import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import org.junit.jupiter.api.DynamicTest;
import org.junit.jupiter.api.TestFactory;
import org.locationtech.jts.geom.Coordinate;
import org.opentripplanner._support.geometry.Polygons;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.street.model.StreetTraversalPermission;
import org.opentripplanner.street.model._data.StreetModelForTest;
import org.opentripplanner.transit.model._data.TransitModelForTest;
import org.opentripplanner.transit.model.framework.Deduplicator;
import org.opentripplanner.transit.model.site.AreaStop;
import org.opentripplanner.transit.service.StopModel;
import org.opentripplanner.transit.service.TransitModel;

class AreaStopsToVerticesMapperTest {

private static final AreaStop BERLIN_AREA_STOP = TransitModelForTest.areaStopForTest(
"berlin",
Polygons.BERLIN
);
public static final StopModel STOP_MODEL = StopModel
.of()
.withAreaStop(AreaStopsToVerticesMapperTest.BERLIN_AREA_STOP)
.build();

public static final TransitModel TRANSIT_MODEL = new TransitModel(STOP_MODEL, new Deduplicator());

private final List<TestCase> testCases = List.of(
new TestCase(BERLIN, ALL, Set.of(BERLIN_AREA_STOP)),
new TestCase(BERLIN, PEDESTRIAN_AND_CAR, Set.of(BERLIN_AREA_STOP)),
new TestCase(BERLIN, BICYCLE_AND_CAR, Set.of()),
new TestCase(HAMBURG, ALL, Set.of()),
new TestCase(BERLIN, PEDESTRIAN, Set.of()),
new TestCase(HAMBURG, PEDESTRIAN, Set.of()),
new TestCase(BERLIN, CAR, Set.of())
);

@TestFactory
Stream<DynamicTest> mapAreaStopsInVertex() {
return testCases
.stream()
.map(tc ->
dynamicTest(
tc.toString(),
() -> {
var graph = new Graph();

var fromVertex = StreetModelForTest.intersectionVertex(tc.coordinate);
var toVertex = StreetModelForTest.intersectionVertex(tc.coordinate);

var edge = StreetModelForTest.streetEdge(fromVertex, toVertex);
edge.setPermission(tc.permission);
fromVertex.addOutgoing(edge);

graph.addVertex(fromVertex);
assertTrue(fromVertex.areaStops().isEmpty());

var mapper = new AreaStopsToVerticesMapper(graph, TRANSIT_MODEL);

mapper.buildGraph();

assertEquals(tc.expectedAreaStops, fromVertex.areaStops());
}
)
);
}

private record TestCase(
Coordinate coordinate,
StreetTraversalPermission permission,
Set<AreaStop> expectedAreaStops
) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private static void addGtfsToGraph(
StreetLinkerModule.linkStreetsForTestOnly(graph, transitModel);

// link flex locations to streets
new FlexLocationsToStreetEdgesMapper(graph, transitModel).buildGraph();
new AreaStopsToVerticesMapper(graph, transitModel).buildGraph();

// generate direct transfers
var req = new RouteRequest();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
import java.util.Locale;
import javax.annotation.Nonnull;
import org.glassfish.jersey.message.internal.OutboundJaxrsResponse;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.params.ParameterizedTest;
import org.opentripplanner._support.time.ZoneIds;
import org.opentripplanner.ext.fares.FaresToItineraryMapper;
Expand Down Expand Up @@ -78,7 +77,6 @@
import org.opentripplanner.transit.service.StopModel;
import org.opentripplanner.transit.service.TransitModel;

@Execution(ExecutionMode.CONCURRENT)
class GraphQLIntegrationTest {

static final Graph graph = new Graph();
Expand All @@ -88,11 +86,12 @@ class GraphQLIntegrationTest {
.toInstant();
static final Instant ALERT_END_TIME = ALERT_START_TIME.plus(1, ChronoUnit.DAYS);

private static final GraphQLRequestContext context;
private static GraphQLRequestContext context;

private static final Deduplicator DEDUPLICATOR = new Deduplicator();

static {
@BeforeAll
static void setup() {
graph
.getVehicleParkingService()
.updateVehicleParking(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,4 @@ public void buildGraph() {
// The bindings are needed to build the request context when routing
graph.dataOverlayParameterBindings = this.parameterBindings;
}

@Override
public void checkInputs() {
//nothing
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package org.opentripplanner.ext.flex;

import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMultimap;
import jakarta.inject.Inject;
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import org.locationtech.jts.geom.Point;
import org.opentripplanner.framework.geometry.GeometryUtils;
import org.opentripplanner.framework.logging.ProgressTracker;
import org.opentripplanner.graph_builder.model.GraphBuilderModule;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.routing.graph.index.StreetIndex;
import org.opentripplanner.street.model.vertex.StreetVertex;
import org.opentripplanner.transit.model.site.AreaStop;
import org.opentripplanner.transit.service.TransitModel;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Iterates over all area stops in the stop model and adds them to vertices that are suitable for
* boarding flex trips.
*/
public class AreaStopsToVerticesMapper implements GraphBuilderModule {

private static final Logger LOG = LoggerFactory.getLogger(AreaStopsToVerticesMapper.class);

private final Graph graph;
private final TransitModel transitModel;

@Inject
public AreaStopsToVerticesMapper(Graph graph, TransitModel transitModel) {
this.graph = graph;
this.transitModel = transitModel;
}

@Override
@SuppressWarnings("Convert2MethodRef")
public void buildGraph() {
if (!transitModel.getStopModel().hasAreaStops()) {
return;
}

StreetIndex streetIndex = graph.getStreetIndexSafe(transitModel.getStopModel());

ProgressTracker progress = ProgressTracker.track(
"Add flex locations to street vertices",
1,
transitModel.getStopModel().listAreaStops().size()
);

LOG.info(progress.startMessage());
var results = transitModel
.getStopModel()
.listAreaStops()
.parallelStream()
.flatMap(areaStop -> {
var matchedVertices = matchingVerticesForStop(streetIndex, areaStop);
// Keep lambda! A method-ref would cause incorrect class and line number to be logged
progress.step(m -> LOG.info(m));
return matchedVertices;
});

ImmutableMultimap<StreetVertex, AreaStop> mappedResults = results.collect(
ImmutableListMultimap.<MatchResult, StreetVertex, AreaStop>flatteningToImmutableListMultimap(
MatchResult::vertex,
mr -> Stream.of(mr.stop())
)
);

mappedResults
.keySet()
.forEach(vertex -> {
vertex.addAreaStops(mappedResults.get(vertex));
});

LOG.info(progress.completeMessage());
}

@Nonnull
private static Stream<MatchResult> matchingVerticesForStop(
StreetIndex streetIndex,
AreaStop areaStop
) {
return streetIndex
.getVerticesForEnvelope(areaStop.getGeometry().getEnvelopeInternal())
.stream()
.filter(StreetVertex.class::isInstance)
.map(StreetVertex.class::cast)
.filter(StreetVertex::isEligibleForCarPickupDropoff)
.filter(vertx -> {
// The street index overselects, so need to check for exact geometry inclusion
Point p = GeometryUtils.getGeometryFactory().createPoint(vertx.getCoordinate());
return areaStop.getGeometry().intersects(p);
})
.map(vertx -> new MatchResult(vertx, areaStop));
}

/**
* The result of an area stop being matched with a vertex.
*/
private record MatchResult(StreetVertex vertex, AreaStop stop) {}
}

This file was deleted.

Loading

0 comments on commit 5e36180

Please sign in to comment.