Skip to content

Commit

Permalink
refactor(Added geojson merging):
Browse files Browse the repository at this point in the history
  • Loading branch information
br648 committed Nov 3, 2023
1 parent 66a9b42 commit 42fa3a1
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@
import com.conveyal.datatools.manager.persistence.Persistence;
import com.conveyal.datatools.manager.utils.ErrorUtils;
import com.conveyal.gtfs.loader.Feed;
import com.conveyal.gtfs.loader.JdbcGtfsExporter;
import com.conveyal.gtfs.loader.Table;
import com.conveyal.gtfs.model.Location;
import com.conveyal.gtfs.model.LocationShape;
import com.conveyal.gtfs.model.StopTime;
import com.conveyal.gtfs.util.GeoJsonUtil;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.Lists;
Expand All @@ -38,13 +42,16 @@
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

import static com.conveyal.datatools.manager.jobs.feedmerge.MergeFeedsType.SERVICE_PERIOD;
import static com.conveyal.datatools.manager.jobs.feedmerge.MergeFeedsType.REGIONAL;
import static com.conveyal.datatools.manager.jobs.feedmerge.MergeStrategy.CHECK_STOP_TIMES;
import static com.conveyal.datatools.manager.models.FeedRetrievalMethod.REGIONAL_MERGE;
import static com.conveyal.datatools.manager.utils.MergeFeedUtils.*;
import static com.conveyal.datatools.manager.utils.MergeFeedUtils.getMergedVersion;
import static com.conveyal.datatools.manager.utils.MergeFeedUtils.stopTimesMatchSimplified;
import static com.conveyal.gtfs.loader.Table.LOCATION_GEO_JSON_FILE_NAME;

/**
* This job handles merging two or more feed versions according to logic specific to the specified merge type.
Expand Down Expand Up @@ -219,13 +226,14 @@ public void jobLogic() {
LOG.error("Merge {} table failed!", table.name);
}
}
mergeLocations(out, feedMergeContext.feedsToMerge);
} catch (IOException e) {
String message = "Error creating output stream for feed merge.";
logAndReportToBugsnag(e, message);
status.fail(message, e);
} finally {
try {
feedMergeContext.close();
if (feedMergeContext != null) feedMergeContext.close();
} catch (IOException e) {
logAndReportToBugsnag(e, "Error closing FeedMergeContext object");
}
Expand All @@ -245,6 +253,26 @@ public void jobLogic() {
}
}

/**
* Merge locations.geojson files. These files are not compatible with the CSV merging strategy. Instead, the
* location.geojson file is flattened into locations and locations shapes. These are then 'merged' based on the
* equal/hash values of each object, converted back into geojson and written to the zip file.
*/
void mergeLocations(ZipOutputStream out, List<FeedToMerge> feedsToMerge) throws IOException {
Set<Location> locations = new HashSet<>();
Set<LocationShape> locationShapes = new HashSet<>();
for (FeedToMerge feed : feedsToMerge) {
ZipEntry locationGeoJsonFile = feed.zipFile.getEntry(LOCATION_GEO_JSON_FILE_NAME);
if (locationGeoJsonFile != null) {
locations.addAll(GeoJsonUtil.getLocationsFromGeoJson(feed.zipFile, locationGeoJsonFile, null));
locationShapes.addAll(GeoJsonUtil.getLocationShapesFromGeoJson(feed.zipFile, locationGeoJsonFile, null));
}
}
if (!locations.isEmpty()) {
JdbcGtfsExporter.writeLocationsToFile(out, new ArrayList<>(locations), new ArrayList<>(locationShapes));
}
}

/**
* Obtains trip ids whose entries in the stop_times table differ between the active and future feed.
*/
Expand Down Expand Up @@ -281,6 +309,10 @@ private boolean shouldSkipTable(String tableName) {
LOG.warn("Skipping editor-only table {}.", tableName);
return true;
}
if (tableName.equals(Table.LOCATIONS.name) || tableName.equals(Table.LOCATION_SHAPES.name)) {
LOG.warn("{} detected. Skipping traditional merge in favour of bespoke merge.", LOCATION_GEO_JSON_FILE_NAME);
return true;
}
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,15 +422,14 @@ public void validateMobility(MonitorableJob.Status status) {
// Wait for the file to be entirely copied into the directory.
// 5 seconds + ~1 second per 10mb
Thread.sleep(5000 + (this.fileSize / 10000));
File gtfsZip = this.retrieveGtfsFile();
// Namespace based folders avoid clash for validation being run on multiple versions of a feed.
// TODO: do we know that there will always be a namespace?
String validatorOutputDirectory = "/tmp/datatools_gtfs/" + this.namespace + "/";

status.update("MobilityData Analysis...", 20);
// Set up MobilityData validator.
ValidationRunnerConfig.Builder builder = ValidationRunnerConfig.builder();
builder.setGtfsSource(gtfsZip.toURI());
builder.setGtfsSource(this.retrieveGtfsFile().toURI());
builder.setOutputDirectory(Path.of(validatorOutputDirectory));
ValidationRunnerConfig mbValidatorConfig = builder.build();

Expand All @@ -442,8 +441,10 @@ public void validateMobility(MonitorableJob.Status status) {
status.update("MobilityData Analysis...", 80);
// Read generated report and save to Mongo.
String json;
try (FileReader fr = new FileReader(validatorOutputDirectory + "report.json")) {
BufferedReader in = new BufferedReader(fr);
try (
FileReader fr = new FileReader(validatorOutputDirectory + "report.json");
BufferedReader in = new BufferedReader(fr)
) {
json = in.lines().collect(Collectors.joining(System.lineSeparator()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ public static void setUp() throws IOException {
*/
@AfterAll
public static void tearDown() {
Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled());
if (project != null) {
project.delete();
}
Auth0Connection.setAuthDisabled(Auth0Connection.getDefaultAuthDisabled());
}

/**
Expand Down Expand Up @@ -139,7 +139,16 @@ void canMergeRegional() throws SQLException {
fakeAgencyWithFlexVersion1.feedLoadResult.stopAreas.rowCount + fakeAgencyWithFlexVersion2.feedLoadResult.stopAreas.rowCount,
"stopAreas count for merged feed should equal sum of area for versions merged."
);
// TODO: locations.geojson.
assertEquals(
mergedVersion.feedLoadResult.locations.rowCount,
fakeAgencyWithFlexVersion1.feedLoadResult.locations.rowCount,
"Merged versions contain the same locations, only one set of locations should remain after merge."
);
assertEquals(
mergedVersion.feedLoadResult.locationShapes.rowCount,
fakeAgencyWithFlexVersion1.feedLoadResult.locationShapes.rowCount,
"Merged versions contain the same location shapes, only one set of location shapes should remain after merge."
);
// Ensure there are no referential integrity errors, duplicate ID, or wrong number of
// fields errors.
assertThatFeedHasNoErrorsOfType(
Expand Down

0 comments on commit 42fa3a1

Please sign in to comment.