Skip to content

Commit

Permalink
refactor(PR feedback): Addressed spelling of GeoJSON and DB connectio…
Browse files Browse the repository at this point in the history
…n handling
  • Loading branch information
br648 committed Nov 28, 2023
1 parent 6da9e99 commit d0317cb
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.io.Serializable;

/** Represents a problem parsing location GeoJson from a GTFS feed. */
/** Represents a problem parsing location GeoJSON from a GTFS feed. */
public class GeoJsonParseError extends GTFSError implements Serializable {
public static final long serialVersionUID = 1L;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public enum NewGTFSErrorType {
FLEX_MISSING_FARE_RULE(Priority.HIGH, "A location zone id must reference a fare rule. One of contains id, destination id or origin id."),
FLOATING_FORMAT(Priority.MEDIUM, "Incorrect floating point number format."),
FREQUENCY_PERIOD_OVERLAP(Priority.MEDIUM, "A frequency for a trip overlaps with another frequency defined for the same trip."),
GEO_JSON_PARSING(Priority.HIGH, "Unable to parse the locations.geojson file. Make sure the file conforms to the GeoJson standard and supported geometry types are used."),
GEO_JSON_PARSING(Priority.HIGH, "Unable to parse the locations.geojson file. Make sure the file conforms to the GeoJSON standard and supported geometry types are used."),
ILLEGAL_FIELD_VALUE(Priority.MEDIUM, "Fields may not contain tabs, carriage returns or new lines."),
INTEGER_FORMAT(Priority.MEDIUM, "Incorrect integer format."),
LANGUAGE_FORMAT(Priority.LOW, "Language should be specified with a valid BCP47 tag."),
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/conveyal/gtfs/loader/Table.java
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ public CsvReader getCsvReader(ZipFile zipFile, SQLErrorStorage sqlErrorStorage)
}

/**
* Create a CSV reader depending on the table to be loaded. If the table is "locations.geojson" unpack the GeoJson
* Create a CSV reader depending on the table to be loaded. If the table is "locations.geojson" unpack the GeoJSON
* data first and load into a CSV reader, else, read the table contents directly into the CSV reader.
*/
public static CsvReader getCsvReader(
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/gtfs/model/Location.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ public void setStatementParameters(PreparedStatement statement, boolean setDefau

/**
* Required by {@link com.conveyal.gtfs.util.GeoJsonUtil#getCsvReaderFromGeoJson(String, ZipFile, ZipEntry, List)} as part
* of the unpacking of GeoJson data to CSV.
* of the unpacking of GeoJSON data to CSV.
*/
public static String header() {
return "location_id,stop_name,stop_desc,zone_id,stop_url,geometry_type\n";
}

/**
* Required by {@link com.conveyal.gtfs.util.GeoJsonUtil#getCsvReaderFromGeoJson(String, ZipFile, ZipEntry, List)} as part
* of the unpacking of GeoJson data to CSV.
* of the unpacking of GeoJSON data to CSV.
*/
public String toCsvRow() {
String stopName = "", stopDesc = "";
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/gtfs/model/LocationShape.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ public void loadOneRow() throws IOException {

/**
* Required by {@link com.conveyal.gtfs.util.GeoJsonUtil#getCsvReaderFromGeoJson(String, ZipFile, ZipEntry, List)}
* as part of the unpacking of GeoJson data to CSV.
* as part of the unpacking of GeoJSON data to CSV.
*/
public static String header() {
return "location_id,geometry_id,geometry_pt_lat,geometry_pt_lon\n";
}

/**
* Required by {@link com.conveyal.gtfs.util.GeoJsonUtil#getCsvReaderFromGeoJson(String, ZipFile, ZipEntry, List)}
* as part of the unpacking of GeoJson data to CSV.
* as part of the unpacking of GeoJSON data to CSV.
*/
public String toCsvRow() {
return String.join(
Expand Down
80 changes: 36 additions & 44 deletions src/main/java/com/conveyal/gtfs/model/StopTime.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package com.conveyal.gtfs.model;

import com.conveyal.gtfs.GTFSFeed;

import org.apache.commons.dbutils.DbUtils;
import org.mapdb.Fun;

import java.io.IOException;
import java.io.Serializable;
import java.sql.Connection;
Expand Down Expand Up @@ -241,49 +238,44 @@ private static boolean flexColumnExist(Connection connection, String tablePrefix
* and DOUBLE_MISSING respectively.
*/
public static List<StopTime> getFlexStopTimesForValidation(Connection connection, String tablePrefix) throws SQLException {
try {
List<StopTime> stopTimes = new ArrayList<>();
if (!flexColumnExist(connection, tablePrefix)) {
return stopTimes;
}
String sql = String.format(
"select id, trip_id, stop_id, arrival_time, departure_time, pickup_type, drop_off_type, " +
"start_pickup_drop_off_window, end_pickup_drop_off_window, mean_duration_factor, mean_duration_offset, " +
"safe_duration_factor, safe_duration_offset " +
"from %sstop_times where " +
"start_pickup_drop_off_window IS NOT NULL " +
"or end_pickup_drop_off_window IS NOT NULL " +
"or mean_duration_factor IS NOT NULL " +
"or mean_duration_offset IS NOT NULL " +
"or safe_duration_factor IS NOT NULL " +
"or safe_duration_offset IS NOT NULL ",
tablePrefix
);
try (PreparedStatement statement = connection.prepareStatement(sql)) {
ResultSet resultSet = statement.executeQuery();
while (resultSet.next()) {
StopTime stopTime = new StopTime();
stopTime.id = resultSet.getInt(1);
stopTime.trip_id = resultSet.getString(2);
stopTime.stop_id = resultSet.getString(3);
stopTime.arrival_time = getIntValue(resultSet.getString(4));
stopTime.departure_time = getIntValue(resultSet.getString(5));
stopTime.pickup_type = resultSet.getInt(6);
stopTime.drop_off_type = resultSet.getInt(7);
stopTime.start_pickup_drop_off_window = getIntValue(resultSet.getString(8));
stopTime.end_pickup_drop_off_window = getIntValue(resultSet.getString(9));
stopTime.mean_duration_factor = getDoubleValue(resultSet.getString(10));
stopTime.mean_duration_offset = getDoubleValue(resultSet.getString(11));
stopTime.safe_duration_factor = getDoubleValue(resultSet.getString(12));
stopTime.safe_duration_offset = getDoubleValue(resultSet.getString(13));
stopTimes.add(stopTime);
}
}
List<StopTime> stopTimes = new ArrayList<>();
if (!flexColumnExist(connection, tablePrefix)) {
return stopTimes;
} finally {
// Close transaction finally.
if (connection != null) DbUtils.closeQuietly(connection);
}
String sql = String.format(
"select id, trip_id, stop_id, arrival_time, departure_time, pickup_type, drop_off_type, " +
"start_pickup_drop_off_window, end_pickup_drop_off_window, mean_duration_factor, mean_duration_offset, " +
"safe_duration_factor, safe_duration_offset " +
"from %sstop_times where " +
"start_pickup_drop_off_window IS NOT NULL " +
"or end_pickup_drop_off_window IS NOT NULL " +
"or mean_duration_factor IS NOT NULL " +
"or mean_duration_offset IS NOT NULL " +
"or safe_duration_factor IS NOT NULL " +
"or safe_duration_offset IS NOT NULL ",
tablePrefix
);
try (PreparedStatement statement = connection.prepareStatement(sql)) {
ResultSet resultSet = statement.executeQuery();
while (resultSet.next()) {
StopTime stopTime = new StopTime();
stopTime.id = resultSet.getInt(1);
stopTime.trip_id = resultSet.getString(2);
stopTime.stop_id = resultSet.getString(3);
stopTime.arrival_time = getIntValue(resultSet.getString(4));
stopTime.departure_time = getIntValue(resultSet.getString(5));
stopTime.pickup_type = resultSet.getInt(6);
stopTime.drop_off_type = resultSet.getInt(7);
stopTime.start_pickup_drop_off_window = getIntValue(resultSet.getString(8));
stopTime.end_pickup_drop_off_window = getIntValue(resultSet.getString(9));
stopTime.mean_duration_factor = getDoubleValue(resultSet.getString(10));
stopTime.mean_duration_offset = getDoubleValue(resultSet.getString(11));
stopTime.safe_duration_factor = getDoubleValue(resultSet.getString(12));
stopTime.safe_duration_offset = getDoubleValue(resultSet.getString(13));
stopTimes.add(stopTime);
}
}
return stopTimes;
}

@Override
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/com/conveyal/gtfs/util/GeoJsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@

/**
* With the aid of this third party library: https://ngageoint.github.io/simple-features-geojson-java/, this util class
* handles the unpacking and packing of GeoJson data. Unpacking flattens the location data into two classes
* handles the unpacking and packing of GeoJSON data. Unpacking flattens the location data into two classes
* {@link Location} and {@link LocationShape}. Packing does the opposite by using these two classes to convert
* the data back into validate GeoJson.
* the data back into validate GeoJSON.
*/
public class GeoJsonUtil {

Expand All @@ -55,7 +55,7 @@ public class GeoJsonUtil {
private static final String STOP_URL = "stop_url";

private GeoJsonUtil() {
throw new IllegalStateException("GeoJson utility class.");
throw new IllegalStateException("GeoJSON utility class.");
}

/**
Expand Down Expand Up @@ -194,9 +194,9 @@ private static String getPropertyValue(Object propertyValue) {

/**
* Extract from a list of features the different geometry types and produce the appropriate {@link LocationShape}
* representing this geometry type so that enough information is available to revert it back to GeoJson.
* representing this geometry type so that enough information is available to revert it back to GeoJSON.
*
* GeoJson format reference: https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.4
* GeoJSON format reference: https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.4
*/
private static List<LocationShape> unpackLocationShapes(
FeatureCollection featureCollection,
Expand Down Expand Up @@ -292,7 +292,7 @@ public static CsvReader getCsvReaderFromGeoJson(
}

/**
* Extract the locations from Geo Json.
* Extract the locations from GeoJSON.
*/
public static List<Location> getLocationsFromGeoJson(ZipFile zipFile, ZipEntry entry, List<String> errors) {
FeatureCollection features = getFeaturesFromGeoJson(zipFile, entry, errors);
Expand All @@ -303,7 +303,7 @@ public static List<Location> getLocationsFromGeoJson(ZipFile zipFile, ZipEntry e
}

/**
* Extract the location shapes from Geo Json.
* Extract the location shapes from GeoJSON.
*/
public static List<LocationShape> getLocationShapesFromGeoJson(ZipFile zipFile, ZipEntry entry, List<String> errors) {
FeatureCollection features = getFeaturesFromGeoJson(zipFile, entry, errors);
Expand All @@ -314,12 +314,12 @@ public static List<LocationShape> getLocationShapesFromGeoJson(ZipFile zipFile,
}

/**
* Extract the Geo Json features from file.
* Extract the GeoJSON features from file.
*/
private static FeatureCollection getFeaturesFromGeoJson(ZipFile zipFile, ZipEntry entry, List<String> errors) {
FeatureCollection features = GeoJsonUtil.getFeatureCollection(zipFile, entry);
if (features == null || features.numFeatures() == 0) {
String message = "Unable to extract GeoJson features (or none are available) from " + entry.getName();
String message = "Unable to extract GeoJSON features (or none are available) from " + entry.getName();
LOG.warn(message);
if (errors != null) errors.add(message);
return null;
Expand All @@ -328,7 +328,7 @@ private static FeatureCollection getFeaturesFromGeoJson(ZipFile zipFile, ZipEntr
}

/**
* Convert {@link Location} and {@link LocationShape} lists to a serialized String conforming to the GeoJson
* Convert {@link Location} and {@link LocationShape} lists to a serialized String conforming to the GeoJSON
* standard.
*/
public static String packLocations(
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/conveyal/gtfs/validator/FlexValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.google.common.collect.Lists;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -52,8 +53,8 @@ public void validate() {

if (isFlexFeed(bookingRules, stopAreas, locations)) {
List<NewGTFSError> errors = new ArrayList<>();
try {
List<StopTime> stopTimes = getFlexStopTimesForValidation(dataSource.getConnection(), feed.databaseSchemaPrefix);
try (Connection connection = dataSource.getConnection()) {
List<StopTime> stopTimes = getFlexStopTimesForValidation(connection, feed.databaseSchemaPrefix);
stopTimes.forEach(stopTime -> errors.addAll(validateStopTime(stopTime, stopAreas, locations)));
feed.trips.forEach(trip -> errors.addAll(validateTrip(trip, stopTimes, stopAreas, locations)));
} catch (SQLException e) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/conveyal/gtfs/GTFSTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ private void assertThatPersistenceExpectationRecordWasFound(
new PersistenceExpectation(
"stop_areas",
new RecordExpectation[]{
new RecordExpectation("area_id", "area1"),
new RecordExpectation("area_id", "area1")
}
),
new PersistenceExpectation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"type" : "FLEX_FORBIDDEN_START_PICKUP_DROP_OFF_WINDOW"
}, {
"count" : 4,
"message" : "Unable to parse the locations.geojson file. Make sure the file conforms to the GeoJson standard and supported geometry types are used.",
"message" : "Unable to parse the locations.geojson file. Make sure the file conforms to the GeoJSON standard and supported geometry types are used.",
"priority" : "HIGH",
"type" : "GEO_JSON_PARSING"
}, {
Expand Down

0 comments on commit d0317cb

Please sign in to comment.